You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2021/02/24 02:15:59 UTC
[GitHub] [groovy] eric-milles opened a new pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
eric-milles opened a new pull request #1498:
URL: https://github.com/apache/groovy/pull/1498
https://issues.apache.org/jira/browse/GROOVY-9951
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on a change in pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1498:
URL: https://github.com/apache/groovy/pull/1498#discussion_r582177312
##########
File path: src/test/groovy/transform/stc/BugsSTCTest.groovy
##########
@@ -938,4 +938,22 @@ Printer
}
'''
}
+
+ // GROOVY-9951
+ void testInnerImmutablePOJO() {
+ assertScript '''
+ import groovy.transform.Immutable
+ import groovy.transform.stc.POJO
+
+ class Outer {
+ @Immutable @POJO
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on a change in pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1498:
URL: https://github.com/apache/groovy/pull/1498#discussion_r581914429
##########
File path: src/test/groovy/transform/stc/BugsSTCTest.groovy
##########
@@ -938,4 +938,22 @@ Printer
}
'''
}
+
+ // GROOVY-9951
+ void testInnerImmutablePOJO() {
+ assertScript '''
+ import groovy.transform.Immutable
+ import groovy.transform.stc.POJO
+
+ class Outer {
+ @Immutable @POJO
Review comment:
AFAIR, `@POJO` should be used with `@CompileStatic`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] paulk-asert commented on pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
paulk-asert commented on pull request #1498:
URL: https://github.com/apache/groovy/pull/1498#issuecomment-786419253
Merged, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] paulk-asert commented on a change in pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
paulk-asert commented on a change in pull request #1498:
URL: https://github.com/apache/groovy/pull/1498#discussion_r582710921
##########
File path: src/main/java/org/apache/groovy/ast/tools/ConstructorNodeUtils.java
##########
@@ -88,17 +91,20 @@ public static Statement checkPropNamesS(final VariableExpression namedArgs, fina
if (!pojo) {
return stmt(callX(IMMUTABLE_TYPE, "checkPropNames", args(varX("this"), namedArgs)));
}
- BlockStatement block = new BlockStatement();
- ListExpression knownPropNames = new ListExpression();
- for (PropertyNode pNode : props) {
- knownPropNames.addExpression(constX(pNode.getName()));
- }
- VariableExpression validNames = localVarX("validNames", ClassHelper.LIST_TYPE);
+
+ Expression validNames = localVarX("validNames", ClassHelper.LIST_TYPE);
Parameter name = param(ClassHelper.STRING_TYPE, "arg");
- Statement loopS = ifS(notX(callX(validNames, "contains", varX(name))),
- throwS(ctorX(EXCEPTION, plusX(constX("Unknown named argument: "), varX(name)))));
- block.addStatement(declS(validNames, knownPropNames));
- block.addStatement(forS(name, callX(namedArgs, "keySet"), loopS));
- return block;
+
+ MethodCallExpression names = callX(namedArgs, "keySet");
+ names.setImplicitThis(false);
+
+ MethodCallExpression isNameValid = callX(validNames, "contains", varX(name));
+ isNameValid.setImplicitThis(false);
+
+ return block(
+ declS(validNames, listX(props.stream().map(p -> constX(p.getName())).collect(toList()))),
+ forS(name, names, ifS(notX(isNameValid),
+ throwS(ctorX(EXCEPTION, plusX(constX("Unknown named argument: "), varX(name))))))
Review comment:
This all LGTM but we can probably go one step further for the plusX expression if `pojo` is true. At the moment it ends up with a call to `StringGroovyMethods#plus` but we could do such a thing without needing the Groovy runtime (for the `pojo` case). There might be multiple ways to do that and we can do that as a next step.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] asfgit closed pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1498:
URL: https://github.com/apache/groovy/pull/1498
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on a change in pull request #1498: GROOVY-9951: set implicit-this to false for method call expressions
Posted by GitBox <gi...@apache.org>.
eric-milles commented on a change in pull request #1498:
URL: https://github.com/apache/groovy/pull/1498#discussion_r582008673
##########
File path: src/test/groovy/transform/stc/BugsSTCTest.groovy
##########
@@ -938,4 +938,22 @@ Printer
}
'''
}
+
+ // GROOVY-9951
+ void testInnerImmutablePOJO() {
+ assertScript '''
+ import groovy.transform.Immutable
+ import groovy.transform.stc.POJO
+
+ class Outer {
+ @Immutable @POJO
Review comment:
This test runs under STC and SC.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org