Skip to main content
3 of 5
corrected another typo
this
  • 2k
  • 13
  • 24

Antlr Visitor with complex result

I'm currently helping out on Rubberduck open source project, 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);
        }
    }
this
  • 2k
  • 13
  • 24