[
https://issues.apache.org/jira/browse/GROOVY-12706?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18139312#comment-18139312
]
ASF GitHub Bot commented on GROOVY-12706:
-----------------------------------------
paulk-asert opened a new pull request, #3236:
URL: https://github.com/apache/groovy/pull/3236
A binding variable resolves to a DynamicVariable, which is not an AST node,
so reading the checker's regex metadata from it made the compiler fail with a
general error instead of reporting the undeclared variable. Read the metadata
only from AST nodes and leave other variables untracked. The
Pattern.matcher(...) and Matcher.group(...) paths are only reached once binding
variables carry checked types (GROOVY-12462); a stub extension standing in for
that covers them.
> RegexChecker fails inside the compiler on script binding variables
> ------------------------------------------------------------------
>
> Key: GROOVY-12706
> URL: https://issues.apache.org/jira/browse/GROOVY-12706
> Project: Groovy
> Issue Type: Bug
> Components: groovy-typecheckers
> Affects Versions: 5.1.3, 6.0.0
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h2. Problem
> With {{groovy.typecheckers.RegexChecker}} enabled, a script that uses a
> binding variable (one assigned without {{def}}) in a regex subscript or a
> {{findGroups}} call makes the compiler fail with a general error, instead of
> reporting the normal type-checking error for an undeclared variable.
> Reproducer, run with {{groovy --configscript config.groovy subscript.groovy}}:
> {code:groovy|title=config.groovy}
> import groovy.transform.TypeChecked
> withConfig(configuration) {
> ast(TypeChecked, extensions: 'groovy.typecheckers.RegexChecker')
> }
> {code}
> {code:groovy|title=subscript.groovy}
> finder = 'groovy' =~ /(oo)/
> finder[0]
> {code}
> Actual:
> {noformat}
> General error during canonicalization: No signature of method:
> getNodeMetaData for class: org.codehaus.groovy.ast.DynamicVariable is
> applicable for argument types: (String) values:
> [RegexChecker_INFERRED_GROUP_COUNT]
> {noformat}
> Expected: the same result as without the extension:
> {noformat}
> [Static type checking] - The variable [finder] is undeclared.
> {noformat}
> ||Script||4.0.32||5.0.6||5.1.3||6.0.0||master||
> |{{finder = 'groovy' =~ /(oo)/; finder[0]}}|general error|general
> error|general error|general error|general error|
> |{{p = ~/(oo)/; 'groovy'.findGroups(p)}}|undeclared (correct)|undeclared
> (correct)|undeclared (correct)|general error|general error|
> |{{p = ~/(oo)/; p.matcher('groovy')}}|undeclared (correct)|undeclared
> (correct)|undeclared (correct)|undeclared (correct)|undeclared (correct)|
> |{{m = 'groovy' =~ /(oo)/; m.group(1)}}|undeclared (correct)|undeclared
> (correct)|undeclared (correct)|undeclared (correct)|undeclared (correct)|
> h2. Cause
> For a binding variable, {{CheckingVisitor.findTargetVariable}} returns an
> {{org.codehaus.groovy.ast.DynamicVariable}}, which is not an {{ASTNode}} and
> has no node metadata. RegexChecker reads its group-count metadata from
> whatever that lookup returns ({{var?.getNodeMetaData(...)}} /
> {{source?.getNodeMetaData(...)}}), so the dynamic {{getNodeMetaData}} call
> fails:
> * the {{matcher[...]}} subscript inference in {{beforeVisitMethod}} (present
> since RegexChecker was added);
> * the {{findGroups}} fallback that takes the group count from a pattern
> variable (6.0.0 onwards);
> * the {{Pattern.matcher(...)}} propagation and the {{Matcher.group(...)}}
> check. These are only reached when the variable's type is {{Pattern}} or
> {{Matcher}}, so they don't fail today. They would once binding variables
> carry checked types (GROOVY-12462).
> The other checkers in groovy-typecheckers that look up variables
> (NullChecker, PurityChecker, ModifiesChecker, SqlInjectionChecker) already
> handle a {{DynamicVariable}} safely.
> h2. Proposed fix
> Treat a lookup result that is not an AST node as an untracked variable: skip
> the regex metadata reads and writes, and leave the variable to normal type
> checking. Cover all four lookup sites, including the {{copyRegexMetaData}}
> helper added by GROOVY-12697.
> h2. Tests
> * Each row of the table above reports only the undeclared-variable error,
> with no general error.
> * A regex declared with {{def}} in the same script is still checked ({{def m
> = 'groovy' =~ /(oo)/; m[0][2]}} reports "Invalid group count 2 for regex with
> 1 group").
> Static compilation of scripts that use binding variables is uncommon today,
> but GROOVY-12462 (typed binding for REPL hosts) is intended to make it
> routine, so this should be fixed before that work.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
