I'm currently helping out on [Rubberduck open source project](https://github.com/rubberduck-vba/Rubberduck/), specifically on the `Extract Method` functionality. I wrote a purpose-built visitor to assist with validating the selection. Basically we have to ensure that when the user select a piece of VBA code, the selection only encompasses a set of complete statements that are also self-contained. By "self-contained", we mean: * it is within a single procedure * does not contain any control-of-flow statements such as `GoTo`, `On Error`, `Exit XXX` that would cause the execution to leave the selection entirely or jump into middle of the selection * each statements are complete (e.g. not partially selected) Note there are 2 issues already present that can be ignored: * Hard-coded literals reporting error. Those need to be in the resource files and will be moved later. I need more thoughts on what I need to collect before I will refactor those. * Currently, the visitor stops as soon as it encounters a invalid context. It will be revised to allow collection of all possible invalid contexts, which is why it's a `IList` rather than a single instance. Specific questions to assess: * To avoid storing a state in the visitor, I opted to create a class `ExtractMethodValidationResult` which importantly has 2 separate lists. `ValidContexts` gives me all contexts (of one type) that are valid for a user's selection whereas `InvalidContexts` gives me any other contexts (regardless of type) where there might be a problem. I then pass that into the generic visitor.In a typical sample, a generic visitor will take a `IEnumerable` of some certain contexts and return that. This is quite different as we take a class and return the results. As a result, the process will create many new instances of the class which is then aggregated. Are there any potential pitfalls with the approach? * I'm not exactly thrilled with the high amount of duplicative code, notably with those that collects invalid contexts. Can this be done better? * I'm a little fuzzy whether I need to return `base.XXX` when visiting contexts that are considered invalid. In case when we visit a invalid context that isn't a part of the selection, we execute the `base.XXX` Any other feedbacks & suggestions welcome. Result code: private class ExtractMethodValidationResult<Result> where Result : ParserRuleContext { public IList<Result> ValidContexts { get; set; } public IList<Tuple<ParserRuleContext, String>> InvalidContexts { get; set; } public ExtractMethodValidationResult() { ValidContexts = new List<Result>(); InvalidContexts = new List<Tuple<ParserRuleContext, String>>(); } public ExtractMethodValidationResult(ParserRuleContext InvalidContext, String ErrorMessage) : this() { InvalidContexts.Add(Tuple.Create(InvalidContext, ErrorMessage)); } } Visitor code: private class ExtractMethodValidatorVisitor : VBAParserBaseVisitor<ExtractMethodValidationResult<VBAParser.BlockStmtContext> { private readonly QualifiedSelection _qualifiedSelection; public ExtractMethodValidatorVisitor(QualifiedSelection qualifiedSelection) { _qualifiedSelection = qualifiedSelection; } protected override ExtractMethodValidationResult<VBAParser.BlockStmtContext> DefaultResult => new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(); public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitBlockStmt([NotNull] VBAParser.BlockStmtContext context) { var children = base.VisitBlockStmt(context); if (children.InvalidContexts.Count == 0) { children.ValidContexts.Add(context); } return children; } protected override ExtractMethodValidationResult<VBAParser.BlockStmtContext> AggregateResult(ExtractMethodValidationResult<VBAParser.BlockStmtContext> aggregate, ExtractMethodValidationResult<VBAParser.BlockStmtContext> nextResult) { if (aggregate.InvalidContexts.Count == 0) { if (nextResult != null) { aggregate.ValidContexts = aggregate.ValidContexts.Concat(nextResult.ValidContexts).ToList(); aggregate.InvalidContexts = aggregate.InvalidContexts.Concat(nextResult.InvalidContexts).ToList(); } } return aggregate; } protected override bool ShouldVisitNextChild(IRuleNode node, ExtractMethodValidationResult<VBAParser.BlockStmtContext> currentResult) { // Don't visit any more children if we have any invalid contexts return (currentResult.InvalidContexts.Count == 0); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitErrorStmt([NotNull] VBAParser.ErrorStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains an Error statement."); } return base.VisitErrorStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitEndStmt([NotNull] VBAParser.EndStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains an End statement."); } return base.VisitEndStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitExitStmt([NotNull] VBAParser.ExitStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains an Exit statement"); } return base.VisitExitStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitGoSubStmt([NotNull] VBAParser.GoSubStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a GoSub statement"); } return base.VisitGoSubStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitGoToStmt([NotNull] VBAParser.GoToStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a GoTo statement"); } return base.VisitGoToStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitOnErrorStmt([NotNull] VBAParser.OnErrorStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a On Error statement"); } return base.VisitOnErrorStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitIdentifierStatementLabel([NotNull] VBAParser.IdentifierStatementLabelContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a Line Label statement"); } return base.VisitIdentifierStatementLabel(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitCombinedLabels([NotNull] VBAParser.CombinedLabelsContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a Line Label statement"); } return base.VisitCombinedLabels(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitOnGoSubStmt([NotNull] VBAParser.OnGoSubStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a On ... GoSub statement"); } return base.VisitOnGoSubStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitOnGoToStmt([NotNull] VBAParser.OnGoToStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a On ... GoTo statement"); } return base.VisitOnGoToStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitResumeStmt([NotNull] VBAParser.ResumeStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a Resume statement"); } return base.VisitResumeStmt(context); } public override ExtractMethodValidationResult<VBAParser.BlockStmtContext> VisitReturnStmt([NotNull] VBAParser.ReturnStmtContext context) { if (_qualifiedSelection.Selection.Contains(context)) { return new ExtractMethodValidationResult<VBAParser.BlockStmtContext>(context, "Extract method cannot extract methods that contains a Return statement"); } return base.VisitReturnStmt(context); } }