[GitHub] groovy pull request #797: GROOVY-8794: Add groovy-yaml subproject to support...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/797#discussion_r217889449
--- Diff: build.gradle ---
@@ -183,6 +185,9 @@ dependencies {
compile("org.apache.ivy:ivy:$ivyVersion") {
transitive = false
}
+compile
"com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:$jacksondataformatyamlVersion"
+compile
"com.fasterxml.jackson.core:jackson-databind:$jacksondatabindVersion"
+
--- End diff --
Just curious why these 2 deps are listed here and not in
`groovy-yaml/build.gradle`?
---
[GitHub] groovy pull request #797: GROOVY-8794: Add groovy-yaml subproject to support...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/797#discussion_r217889473
--- Diff:
subprojects/groovy-yaml/src/main/java/groovy/yaml/YamlBuilder.java ---
@@ -0,0 +1,264 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.yaml;
+
+import groovy.json.JsonBuilder;
+import groovy.lang.Closure;
+import groovy.lang.GroovyObjectSupport;
+import groovy.lang.Writable;
+import groovy.yaml.util.YamlConverter;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.io.Writer;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A builder for creating YAML payloads.
+ *
+ * @since 2.5.3
+ */
+public class YamlBuilder extends GroovyObjectSupport implements Writable {
+private JsonBuilder jsonBuilder;
+
+public YamlBuilder() {
+this.jsonBuilder = new JsonBuilder();
+}
+
+public Object getContent() {
+return jsonBuilder.getContent();
+}
+
+/**
+ * Named arguments can be passed to the YAML builder instance to
create a root YAML object
+ *
+ * Example:
+ *
+ * def yaml = new groovy.yaml.YamlBuilder()
+ * yaml name: "Guillaume", age: 33
+ *
+ * assert yaml.toString() == '{"name":"Guillaume","age":33}'
--- End diff --
Looks like most of the `YamlBuilder#toString` javadoc examples in this
class shows `json` instead of `yaml` output.
---
[GitHub] groovy pull request #757: GROOVY-8008: AIOOB inner class ctor params with ru...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/757#discussion_r196172656
--- Diff: src/main/java/org/codehaus/groovy/vmplugin/v5/Java5.java ---
@@ -405,6 +408,32 @@ public void configureClassNode(CompileUnit
compileUnit, ClassNode classNode) {
}
}
+/**
+ * Synthetic parameters such as those added for inner class
constructors may not be
+ * included in the parameter annotations array. This is the case when
at least one
+ * parameter of an inner class constructor is annotated with a RUNTIME
retention
+ * policy. This method will normalize the annotation array so that it
contains the
+ * same number of elements as the array returned from {@link
Constructor#getParameterTypes()}.
+ *
+ * If adjustment is required, the adjusted array will be pre-pended
will zero-length
--- End diff --
Fixed, thanks.
---
[GitHub] groovy pull request #757: GROOVY-8008: AIOOB inner class ctor params with ru...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/757 GROOVY-8008: AIOOB inner class ctor params with runtime annotations Should also address [GROOVY-8505](https://issues.apache.org/jira/browse/GROOVY-8505) which is the same issue. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8008-aioob-inner-class-ctor-param-annos Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/757.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #757 commit e5e8da40dc3e2d1d141280ee5162247da7ed3083 Author: John Wagenleitner Date: 2018-06-15T02:37:56Z GROOVY-8008: AIOOB inner class ctor params with runtime annotations ---
[GitHub] groovy pull request #756: GROOVY-8614: Invalid reference generated in InnerC...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/756 GROOVY-8614: Invalid reference generated in InnerClasses attribute fo⦠â¦r nested interface You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8614-invalid-inner-nested-interface Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/756.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #756 commit 97c9d108de7197dcdc1aa55a8fc9af18a8880e17 Author: John Wagenleitner Date: 2018-06-15T02:31:59Z GROOVY-8614: Invalid reference generated in InnerClasses attribute for nested interface ---
[GitHub] groovy pull request #753: GROOVY-8632: Groovy 2.5.0 fails to compile Google ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/753 GROOVY-8632: Groovy 2.5.0 fails to compile Google Java Client sample code Class files can contain INNERCLASS references to other classes inner classes whose name may be the same name as a contained inner class. By storing modifiers in a map keyed by short class name there is a possibility for the wrong modifiers to be stored. Since generated class files for inner classes contain an INNERCLASS self reference, the logic can be simplified to look for a matching name and storing those access modifiers. This eliminates the need to search the outer class for the INNERCLASS reference. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8632-decompiled-cn-nested-classes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/753.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #753 commit 8b0d57cfc8c600296763b31a9d9f5166b17ef4f1 Author: John Wagenleitner Date: 2018-06-09T00:00:16Z GROOVY-8632: Groovy 2.5.0 fails to compile Google Java Client sample code Class files can contain INNERCLASS references to other classes inner classes whose name may be the same name as a contained inner class. By storing modifiers in a map keyed by short class name there is a possibility for the wrong modifiers to be stored. Since generated class files for inner classes contain an INNERCLASS self reference, the logic can be simplified to look for a matching name and storing those access modifiers. This eliminates the need to search the outer class for the INNERCLASS reference. ---
[GitHub] groovy pull request #747: GROOVY-8583: Fail to infer auto-return type from t...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/747 GROOVY-8583: Fail to infer auto-return type from ternary operator You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8583-stc-ternary Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/747.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #747 commit 884ddeb4d9aca6c4a68f22f7fbd7ce2ab77f51f1 Author: John Wagenleitner Date: 2018-06-03T02:17:08Z GROOVY-8583: Fail to infer auto-return type from ternary operator ---
[GitHub] groovy pull request #729: GROOVY-8610: STC NPE using DGM collect on Iterator
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/729 GROOVY-8610: STC NPE using DGM collect on Iterator You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8610-stc-collect-iterator Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/729.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #729 commit 137f87a3809e3d8d8f572b07f464237f3277b0b2 Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-27T19:10:59Z GROOVY-8610: STC NPE using DGM collect on Iterator ---
[GitHub] groovy pull request #724: Release 2.5.0 related fixes
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/724 Release 2.5.0 related fixes Some recent commits seem to cause problems with the Nextflow 2.5.0 snapshot builds on the CI server. One can lead to a NPE (commit 90486ae1075d14a62d14452abfea0c27485a67b5) because fieldNode can be reassigned `null` after the `instanceof` check. Another change (commit 57cfd2a3e4d985b3248569ea1d92b02c59627703) seems to have caused an issue with assigning parameterized types. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy rel25fixes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/724.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #724 commit 1376361bb9089b1d54e9938a7055610b67b8df0e Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-24T21:59:20Z Fix parameter matching for parameterized types Revert of change 57cfd2a3e4d985b3 and fixes issues with Nextflow CI builds. commit c7d3e6d8916f78544e7354837fa0ff4b811931f6 Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-25T20:16:18Z Fix NPE if accessed property not a member of the owning class ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/869c365161457c050d5a54c7ff43d73d3263f34e#commitcomment-29066081 thanks! ---
[GitHub] groovy pull request #713: GROOVY-8590: STC incorrectly infers type of nested...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/713 GROOVY-8590: STC incorrectly infers type of nested method call used i⦠â¦n a return stmt You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8590-STC-nested-mc-return Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/713.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #713 commit 69315c3252a77fa325a14c901b5e8edf01bfe2db Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-20T05:32:20Z GROOVY-8590: STC incorrectly infers type of nested method call used in a return stmt ---
[GitHub] groovy pull request #707: GROOVY-8509: SC error call to protected method fro...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/707 GROOVY-8509: SC error call to protected method from same package Because they were closely related, this PR also includes the following: 1. A change to the fix introduced by GROOVY-7883 (b1d1232770aa) to prevent filtering protected methods in the `StaticTypeCheckingVisitor` if caller is in the same package. From the same commit, removes the test `Groovy7883Bug#test3` because it should compile and the new test introduced for GROOVY-8509 in `MethodCallsStaticCompilation` takes it place. 1. A change to revert groovy.sql.Sql#asList (b1d1232770aa) back to `protected`. Fix for GROOVY-7883 enforced visibility for method calls in `TypedChecked` and `CompileStatic` modes. Test was originally put in place to verify the added ClosureParams, so changed test to access method via subclass so that the access modifier for the method can remain protected. I think the spirit of the test is preserved in this case without having to open access to the method under test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8509-SC-protected-same-package Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/707.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #707 commit c76783966f4f9198498abdf6f4696dc36e449cd5 Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-16T14:58:23Z Revert change to groovy.sql.Sql#asList in commit b1d1232770aa Fix for GROOVY-7883 enforced visibility for method calls in TypedChecked and CompileStatic modes. Test was originally put in place to verify the added ClosureParams, so changed test to access method via subclass so that the access modifier for the method can remain protected. commit 8821e8406adeec4d69894b739a2d941c51266c87 Author: John Wagenleitner <jwagenleitner@...> Date: 2018-05-16T15:34:48Z GROOVY-8509: SC error call to protected method from same package Also includes a change to the fix introduced by GROOVY-7883 (b1d1232770aa) to prevent filtering protected methods in the StaticTypeCheckingVisitor if caller is in the same package. From the same commit, removes the test Groovy7883Bug#test3 because it should compile and the new test introduced for GROOVY-8509 in MethodCallsStaticCompilation takes it place. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/b1d1232770aade9672668df4dbc6aa2e2076fa9e#commitcomment-28988363 In subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java: In subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java on line 3976: I think the method access should not be opened to satisfy a test. Usually `@TypeChecked` works similar to dynamic mode for method dispatch so accessing a protected method would normally be allowed. If `@TypeChecked` is changed to enforce visibility rules then maybe better to alter the test to use a sub-class so as to not have to change the access modifier here? ---
[GitHub] groovy pull request #671: GROOVY-8422: Incorrect properties copy in Sql.newI...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/671 GROOVY-8422: Incorrect properties copy in Sql.newInstance The provided Properties should be passed to the DriverManager as-is. A copy is only needed when changes are made to the provided Properties in order to mask sensitive information for logging purposes. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8422-sql-props Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/671.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #671 commit 1f833b4af4e5e5ac667967b55f279e9534d5ec58 Author: John Wagenleitner <jwagenleitner@...> Date: 2018-03-05T16:35:33Z GROOVY-8422: Incorrect properties copy in Sql.newInstance The provided Properties should be passed to the DriverManager as-is. A copy is only needed when changes are made to the provided Properties in order to mask sensitive information for logging purposes. ---
[GitHub] groovy pull request #670: GROOVY-8475: unable to instantiate objects using t...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/670 ---
[GitHub] groovy pull request #670: GROOVY-8475: unable to instantiate objects using t...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/670 GROOVY-8475: unable to instantiate objects using the "new" keyword in groovysh PR #100 was merged into 2_5_X and comments on GROOVY-7562 indicate it wasn't merged into 2_4_X due to binary compatibility concerns. I think this backport addresses those concerns and this PR is targeted only for the 2_4_X branch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8475-groovysh-new Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/670.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #670 commit c220753e867f733ac7c68a56d5a8dd9aa5ac60dc Author: John Wagenleitner <jwagenleitner@...> Date: 2018-03-04T01:53:40Z GROOVY-8475: unable to instantiate objects using the "new" keyword in groovysh Backport fix "GROOVY-7562 Groovysh: Fix custom class instantiation impossible with Interpreter Mode" for the 2_4_X branch. Retain binary compatibilty by retaining and deprecating methods removed in the original fix that was applied to 2_5_X. commit 567bd1e525b8752a7594bf594e86596c9e3f19bb Author: John Wagenleitner <jwagenleitner@...> Date: 2018-03-04T04:21:16Z backport fix GROOVY-7562 for the 2_4_X branch Groovysh: Fix imports not working at all in interpreter mode ---
[GitHub] groovy pull request #563: vmplugin
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/563 ---
[GitHub] groovy pull request #610: GROOVY-8326: @Override should not copied onto meth...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/610 GROOVY-8326: @Override should not copied onto methods generated by ap⦠â¦plying @Memoize You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8326-Memoized-Override Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/610.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #610 commit 8bb23c4a5574925a1eecfe57b1a8acbcafbbcb5a Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-10-01T04:06:49Z GROOVY-8326: @Override should not copied onto methods generated by applying @Memoize ---
[GitHub] groovy pull request #609: GROOVY-8213: Closures are maybe not Threadsafe
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/609 GROOVY-8213: Closures are maybe not Threadsafe To address @blackdrag 's comment in the JIRA I created a JMH benchmark to test a volatile read vs a recheck and sync, https://github.com/jwagenleitner/testing-groovy/tree/master/issues/groovy8213. Based on my runs the volatile read is very close to a normal read. Doing a branched check (without needing to sync) was actually worse in terms of performance. It seems with the `--parallel` feature in Gradle and builds being run on large many core servers this problem seems to be coming up more frequently. See https://github.com/gradle/gradle/issues/1420. While I do believe the publication of the `initialized` variable is an issue and should be addressed, another possible cause of this sort of problem could be related to the way the [`respondTo` methods are implemented by unsetting the `initialized` variable during a call to `super.initialize()` in the `loadMetaInfo() ` method](https://github.com/apache/groovy/blob/7b101dd98ffc04b1bfd2447bbe277340d8954add/src/main/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java#L724-L740). But in this case and the referenced Gradle issue I'm not sure the `respondTo` methods are in play. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8213-Closure-Init-Threadsafe Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/609.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #609 commit 01946dbf2bdd11bb1fa402131ba50fef26f889eb Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-10-01T02:31:16Z GROOVY-8213: Closures are maybe not Threadsafe ---
[GitHub] groovy pull request #605: GROOVY-8220: SC GroovyCastException on parameter f...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/605 GROOVY-8220: SC GroovyCastException on parameter flow typing GROOVY-8157 introduced flow typing for parameters and this fix is required in order to track their assignments in `if` branches for temporary type assignments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8220-ParameterFlowTyping Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/605.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #605 commit ad5074b4db073ebefd1af21676675671a62f6064 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-09-08T00:27:06Z GROOVY-8220: SC GroovyCastException on parameter flow typing GROOVY-8157 introduced flow typing for parameters and this fix is required in order to track their assignments in `if` branches for temporary type assignments. ---
[GitHub] groovy pull request #593: GROOVY-8303: VerifyError for nested class this cal...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/593 GROOVY-8303: VerifyError for nested class this call to static method You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8303-VerifyError-Nested Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/593.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #593 commit 29614bfa2ef1a3c57412b8148125d43b1911dc43 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-08-27T21:23:29Z GROOVY-8303: VerifyError for nested class this call to static method --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #584: GROOVY-8249: Newify local variable declaration fai...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/584 GROOVY-8249: Newify local variable declaration fails to resolve class⦠⦠expression You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8249-Newify-VarDecl-ClassResolve Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/584.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #584 commit 2c11c1b72ff47e6c5ccb2134a7ea86da8cabc028 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-08-14T00:15:00Z GROOVY-8249: Newify local variable declaration fails to resolve class expression --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #583: migrate benchmarks to JMH
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/583 migrate benchmarks to JMH You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy bench2perf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/583.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #583 commit d561bbbe1ef8ff952b9d1297562824e68aca735b Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-23T19:54:48Z migrate benchmarks to JMH --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #582: GROOVY-8269: Unclear definition of default behavio...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/582 GROOVY-8269: Unclear definition of default behavior for trait multipl⦠â¦e inheritance conflicts You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8269-trait-conflict-doc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/582.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #582 commit ca3a10c1e22be899bcd8534a7aee4f5bdaf197d2 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-08-13T19:07:33Z GROOVY-8269: Unclear definition of default behavior for trait multiple inheritance conflicts --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #581: GROOVY-8208: VariableExpressionTransformer does no...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/581 GROOVY-8208: VariableExpressionTransformer does not set source positi⦠â¦on on property expressions You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8208-varxform Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/581.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #581 commit 48ce82bce883ee8bcba777a831ad1ee98b70bf45 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-08-13T04:43:06Z GROOVY-8208: VariableExpressionTransformer does not set source position on property expressions --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #569: include indy in benchmark runs
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/569 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #579: GROOVY-8242: @Newify default attribute value
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/579 GROOVY-8242: @Newify default attribute value Class values are only required for Python-style conversions so the attribute should default to an empty array to indicate it is not strictly required. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8242-Newify-value-default Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/579.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #579 commit e68a7c7ca60e9c6453565bbb91720dfe6479567c Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-08-05T18:05:17Z GROOVY-8242: @Newify default attribute value Class values are only required for Python-style conversions so the attribute should default to an empty array to indicate it is not strictly required. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #576: GROOVY-7995: @CS closure call from closure
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/576 GROOVY-7995: @CS closure call from closure Short syntax of closure call invokes wrong closure if wrapped in another closure. This fix includes a combination of the contributed commit from PR #460 along with the patch (see PR comments) provided by Jochen. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy pr460 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/576.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #576 commit 8916d43f298d26814c664555cae1a800e2cbfe5b Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-22T17:06:21Z GROOVY-7995: @CS closure call from closure Short syntax of closure call invokes wrong closure if wrapped in another closure. This fix includes a combination of the contributed commit from PR #460 along with the patch (see PR comments) provided by Jochen. Thanks for @blindpirate for the contribution. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #575: cache GroovyRunnerRegistry values
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/575 cache GroovyRunnerRegistry values The registry should be read heavy and most use made of the iterator. Few writes/loads should occur, so values should be cached in order to optimize iteration. Benchmarked the change against using a read lock but `volatile` was significantly faster. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy runner-values Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/575.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #575 commit 28576c8da816a970d3a1151744f6b073f374e9a4 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-15T14:00:29Z cache GroovyRunnerRegistry values The registry should be read heavy and most use made of the iterator. Few writes/loads should occur, so values should be cached in order to optimize iteration. commit 43b26fd640a1867c557681d4381113a974cce680 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-17T01:33:20Z GroovyRunnerRegistry iterator benchmarks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #573: add JMH to performance subproject
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/573 add JMH to performance subproject You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy jmh-perf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/573.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #573 commit d1d137ea0191cc9265c1ae444ef58d89a1e876da Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-09T18:49:55Z add JMH to performance subproject --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #572: GROOVY-8251: Implement withCloseable on AutoClosea...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/572 GROOVY-8251: Implement withCloseable on AutoCloseable `NioGroovyMethods.withAutoCloseable` was added for 2.5.0 as part of [GROOVY-7572](https://issues.apache.org/jira/browse/GROOVY-7572). It appears it was originally placed here because at the time Java 6 was still used, but now 2.5.0 is 7+. First commit proposes moving the method (as-is) into the core `IOGroovyMethods` class. This is where the `withCloseable(java.io.Closeable...)` method is, so also part of this change is moving the tests for it from NIO to core. Since 2.5.0 is not been officially released I don't think there is a need to deprecate or worry about binary compatibility for the `withAutoCloseable` method. Second commit proposes to rename the new but unreleased method from `withAutoCloseable` to `withCloseable` to match the existing method on `java.io.Closeable`. See discussion on [GROOVY-8251](https://issues.apache.org/jira/browse/GROOVY-8251). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8251-AutoCloseable Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/572.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #572 commit 912200153193849fc35a09a053898d46c7f1dacd Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-09T00:22:36Z GROOVY-8251: Implement withCloseable on AutoCloseable Relocate withAutoCloseable from NIO subproject to core. Since AutoCloseable is not an NIO related class and release 2.5.0 will target Java 7, the method should be available as part of core. Relocated withCloseable tests to core since that method is deprecated in the NIO module. commit 261bc174980da8d6b6f4cdb54b8f0ab98c64c4fd Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-07-09T01:13:48Z GROOVY-8251: rename withAutoCloseable to withCloseable --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/559#discussion_r126288792
--- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java ---
@@ -1218,7 +1218,7 @@ public void
visitVariableExpression(VariableExpression expression) {
BytecodeVariable variable =
controller.getCompileStack().getVariable(variableName, false);
if (variable == null) {
-processClassVariable(variableName);
+processClassVariable(expression);
--- End diff --
Thanks for the PR. I merged before seeing the comment from @blackdrag, so
another refactor PR would be good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/559#discussion_r126286817
--- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java ---
@@ -1251,11 +1251,13 @@ private void processClassVariable(String name) {
mv.visitInsn(DUP);
loadThisOrOwner();
-mv.visitLdcInsn(name);
+mv.visitLdcInsn(expression.getName());
mv.visitMethodInsn(INVOKESPECIAL,
"org/codehaus/groovy/runtime/ScriptReference", "",
"(Lgroovy/lang/Script;Ljava/lang/String;)V", false);
} else {
-PropertyExpression pexp = new PropertyExpression(new
VariableExpression("this"), name);
+PropertyExpression pexp = new PropertyExpression(new
VariableExpression("this"), expression.getName());
--- End diff --
Scratch that comment, I was looking at the wrong source.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/559#discussion_r126286728
--- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java ---
@@ -1251,11 +1251,13 @@ private void processClassVariable(String name) {
mv.visitInsn(DUP);
loadThisOrOwner();
-mv.visitLdcInsn(name);
+mv.visitLdcInsn(expression.getName());
mv.visitMethodInsn(INVOKESPECIAL,
"org/codehaus/groovy/runtime/ScriptReference", "",
"(Lgroovy/lang/Script;Ljava/lang/String;)V", false);
} else {
-PropertyExpression pexp = new PropertyExpression(new
VariableExpression("this"), name);
+PropertyExpression pexp = new PropertyExpression(new
VariableExpression("this"), expression.getName());
--- End diff --
The patch provided in the JIRA creates a new `VariableExpression` (see
below), I think that needs to be done so the source position is not set on the
constant `VariableExpress.THIS_EXPRESSION` via the
`getObjectExpression().setSourcePosition(..)` call.
```java
PropertyExpression pexp = new PropertyExpression(new
VariableExpression("this", ClassHelper.DYNAMIC_TYPE), name);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/558#discussion_r12008
--- Diff:
src/main/org/codehaus/groovy/transform/SortableASTTransformation.java ---
@@ -82,6 +82,7 @@ public void visit(ASTNode[] nodes, SourceUnit source) {
private void createSortable(AnnotationNode annotation, ClassNode
classNode) {
List includes = getMemberStringList(annotation,
"includes");
List excludes = getMemberStringList(annotation,
"excludes");
+boolean reversed = getMemberBoolValue(annotation, "reversed");
--- End diff --
`boolean reversed = memberHasValue(annotation, "reversed", true);`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/558#discussion_r125553175
--- Diff:
src/main/org/codehaus/groovy/transform/SortableASTTransformation.java ---
@@ -112,7 +113,7 @@ private static void implementComparable(ClassNode
classNode) {
}
}
-private static Statement createCompareToMethodBody(List
properties) {
+private static Statement createCompareToMethodBody(List
properties, Boolean reversed) {
--- End diff --
`boolean` as the parameter type would be consistent with the other methods
below.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/558#discussion_r125552693
--- Diff: src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java ---
@@ -219,10 +219,38 @@ public static ClosureExpression closureX(Statement
code) {
return result;
}
+/**
+ * Build a binary expression that compares two values
+ * @param lhv expression for the value to compare from
+ * @param rhv expression for the value value to compare to
+ * @return the expression comparing two values
+ */
public static BinaryExpression cmpX(Expression lhv, Expression rhv) {
return new BinaryExpression(lhv, CMP, rhv);
}
+/**
+ * Build a binary expression that compares two values
+ * @param lhv expression for the value to compare from
+ * @param rhv expression for the value value to compare to
+ * @param reversed whether to use natural ordering or reversed natural
ordering
+ * @return the expression comparing two values
+ */
+public static BinaryExpression cmpX(Expression lhv, Expression rhv,
boolean reversed) {
+return (reversed) ? cmpXReversed(lhv, rhv) : cmpX(lhv, rhv);
--- End diff --
What about `return (reversed) ? cmpX(rhv, lhv) : cmpX(lhv, rhv);`? Could
eliminate the need for the new `cmpXReversed` method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #569: include indy in benchmark runs
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/569 include indy in benchmark runs You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy benchmarks Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/569.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #569 commit ba98d61628f4b2d284e4edaf3053a7adc347bd29 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-28T00:45:23Z include indy in benchmark runs --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #564: GROOVY-8197: Make JUnit3/4 GroovyRunners
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/564 GROOVY-8197: Make JUnit3/4 GroovyRunners You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8197-groovyrunners Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/564.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #564 commit 469f0e970999e84d1e878801ac56b22af73b9cbf Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-09T22:41:38Z GROOVY-8197: Make JUnit3/4 GroovyRunners --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #563: vmplugin
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/563 vmplugin Alternative to PR #562. I tried to make incremental commits to make it easier to review. Still outstanding are the indy related classes in `org.codehaus.groovy.vmplugin.v7`. Just not sure what a good package name for those would be, maybe `org.apache.groovy.internal.runtime.indy`? You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy vmplugin25 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/563.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #563 commit 0ee306754fa0dcfd187999228c4a414bd5db3457 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:26:33Z move Java8 plugin to new org.apache.groovy.internal.vmplugin package commit 26e0d492329a6081ead13a3cd943fd1a7a3a8703 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:27:20Z move core VMPlugin classes to new package commit d07feef77ea29fc0de5a2b6537a67f9f7302712b Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:27:43Z extract Java5 plugin into VMPluginBase class commit 9329434f22ff4ff4569535cf882a2372fc366f54 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:28:31Z use new VMPluginFactory commit 55dbde979a1a7ef83979e535812517d08896b4dc Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:28:47Z deprecate Java6 plugin commit a80fea8d07f8c722de2c37b593b3d72c16b02b6f Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:29:04Z Java7 extends VMPluginBase commit 0513559f52a33584018a85834b8158d781d12a29 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:29:29Z deprecate Java5 plugin default methods commit 8072c5f64e09378d571f8a565507a148f2b3e536 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:30:04Z deprecate Java5 JUnit4Utils commit 5fdc11f8339171054b060de27275130e47474eaa Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:30:15Z move Java7 VM plugin to new package commit f9517054093d064c8a59e1b67ad63ac4e461c409 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-20T23:30:37Z extract Java7 methods into base --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #562: rename Java8 package from vm8 to v8 for consistenc...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/562 rename Java8 package from vm8 to v8 for consistency You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy v8 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/562.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #562 commit f963d8fa08d8d61ac5e9c01e809b617fe45931c9 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-19T01:06:29Z rename Java8 package from vm8 to v8 for consistency --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #561: GROOVY-8230: Deadlock in GroovyClassLoader
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/561 GROOVY-8230: Deadlock in GroovyClassLoader You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8230-gcl-deadlock Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/561.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #561 commit cc2ab1a5fe09d56de905d0b0a3b3a87fcf0d6a60 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-19T00:16:07Z GROOVY-8230: Deadlock in GroovyClassLoader --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #560: GROOVY-8226: JSR308 initial plumbing tweaks
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/560#discussion_r121416370
--- Diff: src/main/org/codehaus/groovy/vmplugin/vm8/Java8.java ---
@@ -51,4 +53,13 @@ public int getVersion() {
return 8;
}
+protected int getElementCode(ElementType value) {
--- End diff --
might be good to have `@Override`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #557: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/557 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #500: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/500 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #557: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/557 GROOVY-8056: GroovyCodeSource(URL) can leak a file handler A safer fix in terms of compatibility compared to PR #500. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8056-urlcon-leak Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/557.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #557 commit d99ff70a729448d75d3ca5b98c70733ba1ca428a Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-06-03T14:50:41Z GROOVY-8056: GroovyCodeSource(URL) can leak a file handler --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #553: require JDK 8+ for release builds (2_5_X/2_6_X)
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/553 require JDK 8+ for release builds (2_5_X/2_6_X) For 2_5_X and 2_6_X in order to compile all features JDK 8+ is required for building releases (related to PR #545). Also included a commit to add JSR223 DGMs in the GDK docs. These methods were moved from `org.codehaus.groovy.vmplugin.v6.PluginDefaultGroovyMethods` and `org.codehaus.groovy.vmplugin.v6.PluginStaticGroovyMethods` but the docs build was not updated to include the new files. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 7611-build-25x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/553.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #553 commit 2a2de466152437e0ca97dfd70e3aaf0cae03d3f6 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-27T22:21:43Z require JDK 8+ for release builds Related to change for GROOVY-7611 (PR #545), JDK 8+ is required in order to compile all features. commit 5764f17eeb5f6b5b33b27480dc09294db487368d Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-27T22:23:58Z include JSR223 DGMs in generated GDK docs --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #545: GROOVY-7611: java.util.Optional should evaluate to...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/545 GROOVY-7611: java.util.Optional should evaluate to false if empty (Java8 VMPlugin) Target for this would be `2_5_X` and above. For `2_5_X` and `2_6_X` it would require that the release process build with JDK 8 so would need to backport some of the build checks for Java version to those branches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 7611-Optional Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/545.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #545 commit 720dced06b1ec447d77b00550b8b299dfe3e18d9 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-21T17:16:06Z GROOVY-7611: java.util.Optional should evaluate to false if empty --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #544: refactor: type safety and formatting
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/544#discussion_r117615134
--- Diff: src/main/org/codehaus/groovy/transform/trait/Traits.java ---
@@ -355,19 +355,19 @@ static String getSuperTraitMethodName(ClassNode
trait, String method) {
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
-public static @interface Implemented {}
+public @interface Implemented {}
/**
* Internal annotation used to indicate that a method is a bridge
method to a trait
* default implementation.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
- public static @interface TraitBridge {
--- End diff --
The `static` modifier is redundant on inner interfaces. The bytecode will
remain unchanged with or without the modifier so the code referenced should
continue to work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #544: refactor: type safety and formatting
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/544 refactor: type safety and formatting You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy refactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/544.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #544 commit 71da9595ff1b3cda9534208620025efd739a1455 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-20T16:00:23Z refactor: type safety commit 5f2642b09f2625177b962ce4faa805e86229185d Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-20T16:35:40Z refactor: formatting and type safety --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/b02d2f57c09ba6ba46553ed77aff191ec9ab274e#commitcomment-22218912 In src/main/org/apache/groovy/util/Maps.java: In src/main/org/apache/groovy/util/Maps.java on line 11: JDK9 added `Map.ofEntries` to handle this and also added `Map.of` which is defined for up to 10 elements and both are type safe. All uses of this will not be type safe. Even though this is used internally, it would be nice to have a type safe way to build maps and not use raw types. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #543: GROOVY-8166: Repeated operations in AnnotationColl...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/543 GROOVY-8166: Repeated operations in AnnotationCollectorTransform and ⦠â¦Traits You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8166 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/543.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #543 commit 72c01601d4cdd00622e6215b2a8d8dcac2ca4a89 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-19T19:48:53Z GROOVY-8166: Repeated operations in AnnotationCollectorTransform and Traits --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r117078128
--- Diff: src/main/groovy/lang/MetaClassImpl.java ---
@@ -1832,6 +1832,9 @@ public Object getProperty(Class sender, Object
object, String name, boolean useS
} catch (IllegalArgumentException e) {
// can't access the field directly but there may be a
getter
mp = null;
+} catch (GroovyRuntimeException e) {
+// can't access the field directly but there may be a
getter
+mp = null;
--- End diff --
I don't understand why this was added. Was there a test that failed
because of the other changes that required this. I would have assumed an
`AccessControlException` from `CachedField.getProperty` would be treated
similar to the `IllegalAccessException` in the same method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/533#discussion_r116533820
--- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java ---
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.ast.tools;
+
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+
+public class MethodNodeUtils {
+/**
+ * Return the method node's descriptor including its
+ * name and parameter types without generics.
+ *
+ * @param mNode the method node
+ * @return the method node's abbreviated descriptor excluding the
return type
+ */
+public static String methodDescriptorWithoutReturnType(MethodNode
mNode) {
--- End diff --
That is what is sort of confusing, the method descriptor for bytecode vs
the method signature used to detect duplicate signatures. Agree that the
formatting is particular to it's use in the `Verifier` and having in a separate
utils class is probably the best approach.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #540: GROOVY-7535: Groovy category throwing MissingMetho...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/540 GROOVY-7535: Groovy category throwing MissingMethodException and Miss⦠â¦ingPropertyException when using multiple threads You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy7535 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/540.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #540 commit 1c2ca41491fd919ddc1c543ec92a8ab7ca3b860b Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-05-14T01:51:47Z GROOVY-7535: Groovy category throwing MissingMethodException and MissingPropertyException when using multiple threads --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365638
--- Diff:
src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java ---
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.reflection;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ReflectPermission;
+
+import groovy.lang.GroovyObject;
+
+class AccessPermissionChecker {
--- End diff --
Might be good to add a private constructor to signal that no instances of
this method are desired, since it only contains static methods.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365514
--- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java ---
@@ -324,6 +337,12 @@ else if (o2 instanceof CachedMethod)
}
public Method getCachedMethod() {
+try {
+AccessPermissionChecker.checkAccessPermission(cachedMethod);
+}
+catch (AccessControlException ex) {
+throw new IllegalArgumentException("Illegal access to method"
+ cachedMethod.getName());
--- End diff --
see comment on `setAccessible`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365223
--- Diff: src/main/org/codehaus/groovy/reflection/CachedField.java ---
@@ -65,6 +72,12 @@ public Object getProperty(final Object object) {
* @throws RuntimeException if the property could not be set
*/
public void setProperty(final Object object, Object newValue) {
+try {
+AccessPermissionChecker.checkAccessPermission(field);
+}
+catch (AccessControlException ex) {
+throw new IllegalArgumentException("Illegal access to field" +
" " + field.getName());
--- End diff --
see above comment about `GroovyRuntimeException`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365151
--- Diff:
src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java ---
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.reflection;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ReflectPermission;
+
+import groovy.lang.GroovyObject;
+
+class AccessPermissionChecker {
+
+ private static final ReflectPermission REFLECT_PERMISSION = new
ReflectPermission("suppressAccessChecks");
+
+ static private void checkAccessPermission(Class declaringClass,
final int modifiers, boolean isAccessible) {
+ final SecurityManager securityManager =
System.getSecurityManager();
+ if (isAccessible && securityManager != null) {
+ if ((modifiers & Modifier.PRIVATE) != 0
+ || ((modifiers &
(Modifier.PUBLIC | Modifier.PROTECTED)) == 0
+&&
packageCanNotBeAddedAnotherClass(declaringClass))
+ &&
!GroovyObject.class.isAssignableFrom(declaringClass)) {
+
securityManager.checkPermission(REFLECT_PERMISSION);
+}
+else if ((modifiers & (Modifier.PROTECTED)) != 0
+ &&
declaringClass.equals(ClassLoader.class)){
+
securityManager.checkCreateClassLoader();
+ }
+ }
+ }
+
+ private static boolean packageCanNotBeAddedAnotherClass(Class
declaringClass) {
+ return declaringClass.getName().startsWith("java.");
+ }
+
+ static public void checkAccessPermission(Method method) {
+ checkAccessPermission(method.getDeclaringClass(),
method.getModifiers(), method.isAccessible()
+ );
+ }
+
+ public static void checkAccessPermission(Field field) {
--- End diff --
`public` isn't needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365503
--- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java ---
@@ -124,6 +131,12 @@ public String getSignature() {
}
public final Method setAccessible() {
+try {
+AccessPermissionChecker.checkAccessPermission(cachedMethod);
+}
+catch (AccessControlException ex) {
+throw new IllegalArgumentException("Illegal access to method"
+ cachedMethod.getName());
--- End diff --
No arguments to this method so `IllegalArgumentException` doesn't seem
right. Think it might be good to either just remove the `try/catch` or wrap in
a `GroovyRuntimeException` (to capture cachedMethod.getName).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365354
--- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java ---
@@ -90,6 +91,12 @@ public CachedClass getDeclaringClass() {
public final Object invoke(Object object, Object[] arguments) {
try {
+AccessPermissionChecker.checkAccessPermission(cachedMethod);
+}
+catch (AccessControlException ex) {
+throw new InvokerInvocationException(new
IllegalArgumentException("Illegal access to method" + cachedMethod.getName()));
--- End diff --
Throwing an `IllegalArgumentException` could be confusing since it's not
the `arguments` passed to the invoked method causing the problem. Maybe
`throw new InvokerInvocationException(ex)`
or
```
throw new InvokerInvocationException(
new AccessControlException("Illegal access to method" +
cachedMethod.getName(), ex)
)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365210
--- Diff: src/main/org/codehaus/groovy/reflection/CachedField.java ---
@@ -51,6 +52,12 @@ public int getModifiers() {
*/
public Object getProperty(final Object object) {
try {
+AccessPermissionChecker.checkAccessPermission(field);
+}
+catch (AccessControlException ex) {
+throw new IllegalArgumentException("Illegal access to field" +
" " + field.getName());
--- End diff --
Any reason for using `IllegalArgumentException`? I think the following may
be more appropriate:
```java
throw new GroovyRuntimeException("Illegal access to field " +
field.getName() + ".", ex);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365112
--- Diff:
src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java ---
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.reflection;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ReflectPermission;
+
+import groovy.lang.GroovyObject;
+
+class AccessPermissionChecker {
+
+ private static final ReflectPermission REFLECT_PERMISSION = new
ReflectPermission("suppressAccessChecks");
+
+ static private void checkAccessPermission(Class declaringClass,
final int modifiers, boolean isAccessible) {
+ final SecurityManager securityManager =
System.getSecurityManager();
--- End diff --
Might be good to exit early if `securityManager == null`, then wont have to
check again below.
Also, be good to change order to be `private static void ...`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/532#discussion_r116365147
--- Diff:
src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java ---
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.reflection;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ReflectPermission;
+
+import groovy.lang.GroovyObject;
+
+class AccessPermissionChecker {
+
+ private static final ReflectPermission REFLECT_PERMISSION = new
ReflectPermission("suppressAccessChecks");
+
+ static private void checkAccessPermission(Class declaringClass,
final int modifiers, boolean isAccessible) {
+ final SecurityManager securityManager =
System.getSecurityManager();
+ if (isAccessible && securityManager != null) {
+ if ((modifiers & Modifier.PRIVATE) != 0
+ || ((modifiers &
(Modifier.PUBLIC | Modifier.PROTECTED)) == 0
+&&
packageCanNotBeAddedAnotherClass(declaringClass))
+ &&
!GroovyObject.class.isAssignableFrom(declaringClass)) {
+
securityManager.checkPermission(REFLECT_PERMISSION);
+}
+else if ((modifiers & (Modifier.PROTECTED)) != 0
+ &&
declaringClass.equals(ClassLoader.class)){
+
securityManager.checkCreateClassLoader();
+ }
+ }
+ }
+
+ private static boolean packageCanNotBeAddedAnotherClass(Class
declaringClass) {
+ return declaringClass.getName().startsWith("java.");
+ }
+
+ static public void checkAccessPermission(Method method) {
--- End diff --
`public` isn't needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/533#discussion_r116363909
--- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java ---
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.ast.tools;
+
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+
+public class MethodNodeUtils {
+/**
+ * Return the method node's descriptor including its
+ * name and parameter types without generics.
+ *
+ * @param mNode the method node
+ * @return the method node's abbreviated descriptor excluding the
return type
+ */
+public static String methodDescriptorWithoutReturnType(MethodNode
mNode) {
--- End diff --
This seems like it could be an instance method such as
`MethodNode#getSignature()` ([JLS
8.4.2](http://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.4.2))
or `MethodNode#getTypeDescriptorWithoutReturnType()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/533#discussion_r116363829
--- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java ---
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.ast.tools;
+
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+
+public class MethodNodeUtils {
+/**
+ * Return the method node's descriptor including its
+ * name and parameter types without generics.
+ *
+ * @param mNode the method node
+ * @return the method node's abbreviated descriptor excluding the
return type
+ */
+public static String methodDescriptorWithoutReturnType(MethodNode
mNode) {
+StringBuilder sb = new StringBuilder();
+mNode.getTypeDescriptor();
+sb.append(mNode.getName()).append(':');
+for (Parameter p : mNode.getParameters()) {
+
sb.append(ClassNodeUtils.formatTypeName(p.getType())).append(',');
+}
+return sb.toString();
+}
+
+/**
+ * Return the method node's descriptor which includes its return type,
+ * name and parameter types without generics.
+ *
+ * @param mNode the method node
+ * @return the method node's descriptor
+ */
+public static String methodDescriptor(MethodNode mNode) {
--- End diff --
This could be the body of `MethodNode#getTypeDescriptor` instance method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/533#discussion_r116363767
--- Diff: src/main/org/apache/groovy/ast/tools/ClassNodeUtils.java ---
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.ast.tools;
+
+import org.codehaus.groovy.ast.ClassNode;
+
+public class ClassNodeUtils {
+/**
+ * Formats a type name into a human readable version. For arrays,
appends "[]" to the formatted
+ * type name of the component. For unit class nodes, uses the class
node name.
+ *
+ * @param cNode the type to format
+ * @return a human readable version of the type name
(java.lang.String[] for example)
+ */
+public static String formatTypeName(ClassNode cNode) {
--- End diff --
This seems like a candidate to be a package-private instance method on
`ClassNode`. Package-private part assumes the moving of the `MethodNodeUtils`
methods into `MethodNode`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/0fb89906aa587920d11fae063bba1d1f8fe26254#commitcomment-21963417 Thanks, I had forgot to test with antlr4 enabled. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #531: fix spec test
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/531 fix spec test You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy dollarslashy Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/531.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #531 commit ffe47e95d766359833a4463fe3d8ad26dafcd17f Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-30T20:14:24Z fix spec test --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #530: Clarify documentation around indy
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/530 Clarify documentation around indy From a recent [user mailing list question](http://mail-archives.apache.org/mod_mbox/groovy-users/201704.mbox/%3c44315a6eb40a3769a12a5bfe465da...@posteo.de%3E) and related to PR groovy/groovy-website#124. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy indy-docs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/530.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #530 commit 055d1013e66b95dc18c2b367ffb3d797e67c9639 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-30T18:07:34Z Clarify documentation around indy --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #528: GROOVY-7579: Improve docs for invokeMethod
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/528 GROOVY-7579: Improve docs for invokeMethod You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy7579 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/528.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #528 commit 79cd42d410310f6d2ea34a13e78d621dc2e658a9 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-23T23:06:13Z GROOVY-7579: Improve docs for invokeMethod --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #524: GROOVY-8156: Compile error when ListenerList annot...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/524 GROOVY-8156: Compile error when ListenerList annotation exists You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8156 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/524.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #524 commit 1d24e37353ef1544a2d5a6fdb7929b1a46c598f3 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-14T20:33:59Z GROOVY-8156: Compile error when ListenerList annotation exists --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #523: cleanup now that jdk7 is baseline
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/523 cleanup now that jdk7 is baseline Avoids reflective checks/constructions where it is no longer needed since 7 is now baseline in 2_5_X and master. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy cleanup-jdk7 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/523.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #523 commit 0982b7927f463cc98f8019576a7e1375e8d7a2c6 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-12T02:43:14Z cleanup now that jdk7 is baseline --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #522: GROOVY-8144: Invoking a public method declared in ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/522 GROOVY-8144: Invoking a public method declared in a non-public class ⦠â¦result in a IllegalAccessError Commit 1a4c9918a4f12e64 introduced the DecompiledClassNode as part of enabling the ASM class resolver. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8144 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/522.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #522 commit 3b6c2a504119e99d86de76019f48e0059b8fcf9e Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-10T20:10:25Z GROOVY-8144: Invoking a public method declared in a non-public class result in a IllegalAccessError Commit 1a4c9918a4f12e64 introduced the DecompiledClassNode as part of enabling the ASM class resolver. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #520: GROOVY-8140: Invoke method not returning MOP super...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/520 GROOVY-8140: Invoke method not returning MOP super method if isCallTo⦠â¦Super See [dev mailing list discussion](http://mail-archives.apache.org/mod_mbox/groovy-dev/201703.mbox/%3CCAPbPdObHqPg7aFHSwLf0sRMdHqrRa1vuNwRvxuZVQu5VQ840%2Bw%40mail.gmail.com%3E) You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8140 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/520.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #520 commit 433eaf8c62f809d531bb9984b126252172f8287c Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-04-02T15:34:47Z GROOVY-8140: Invoke method not returning MOP super method if isCallToSuper --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #500: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/500 GROOVY-8056: GroovyCodeSource(URL) can leak a file handler URLConnect.getContentEncoding returns the Content-Encoding HTTP Header [1] which is not a charset. Since this method would have either returned null or an invalid charset, the code path specifying the encoding would normally not have been executed. The charset may be contained in the Content-Type header, but rather than attempt to parse that string which would require closing the connection, this fix avoids opening the connection and relies on the default charset. [1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11 You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8056-content-encoding Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/500.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #500 commit 29a641ccc212d397bdc11d3a995763b88dfe34b5 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-02-19T00:22:49Z GROOVY-8056: GroovyCodeSource(URL) can leak a file handler URLConnect.getContentEncoding returns the Content-Encoding HTTP Header [1] which is not a charset. Since this method would have either returned null or an invalid charset, the code path specifying the encoding would normally not have been executed. The charset may be contained in the Content-Type header, but rather than attempt to parse that string which would require closing the connection, this fix avoids opening the connection and relies on the default charset. [1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #496: Add a ASMifier tab to AstBrowser
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/496#discussion_r100702338
--- Diff:
subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstBrowser.groovy
---
@@ -403,9 +420,13 @@ class AstBrowser {
def tabPane = mainSplitter.bottomComponent
int tabCount = tabPane.getTabCount()
for (int i = 0; i < tabCount; i++) {
-if (bytecodeView.is(tabPane.getComponentAt(i))) {
+def component = tabPane.getComponentAt(i);
+if (bytecodeView.is(component)) {
tabPane.setTitleAt(i, getByteCodeTitle())
break
+} else if (asmifierView.is(component)) {
+tabPane.setTitleAt(i, getASMifierTitle())
+break
--- End diff --
The `ASMifier` tab title doesn't update because of the `breaks`, if both
`breaks` are removed that should fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #489: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/489#discussion_r100162935
--- Diff:
subprojects/stress/src/test/java/org/codehaus/groovy/reflection/ClassInfoDeadlockStressTest.java
---
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.reflection;
+
+import groovy.lang.GroovyClassLoader;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.groovy.stress.util.GCUtils;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+/**
+ * Tests for deadlocks in the ClassInfo caching.
+ *
+ */
+public class ClassInfoDeadlockStressTest {
+
+private static final int DEADLOCK_TRIES = 8;
+private static final int THREAD_COUNT = 8;
+
+private final CountDownLatch startLatch = new CountDownLatch(1);
+private final CountDownLatch completeLatch = new
CountDownLatch(THREAD_COUNT);
+private final GroovyClassLoader gcl = new GroovyClassLoader();
+private final AtomicInteger counter = new AtomicInteger();
+
+/**
+ * We first generate a large number of ClassInfo instances for classes
+ * that are no longer reachable. Then queue up threads to all request
+ * ClassInfo instances for new classes simultaneously to ensure that
+ * clearing the old references wont deadlock the creation of new
+ * instances.
+ *
+ * GROOVY-8067
+ */
+@Test
+public void testDeadlock() throws Exception {
+for (int i = 1; i <= DEADLOCK_TRIES; i++) {
+System.out.println("Test Number: " + i);
+generateGarbage();
+GCUtils.gc();
+attemptDeadlock(null);
+}
+}
+
+@Test
+public void testRequestsForSameClassInfo() throws Exception {
+Class newClass = createRandomClass();
+for (int i = 1; i <= DEADLOCK_TRIES; i++) {
+System.out.println("Test Number: " + i);
+generateGarbage();
+GCUtils.gc();
+attemptDeadlock(newClass);
+}
+ClassInfo newClassInfo = ClassInfo.getClassInfo(newClass);
+for (ClassInfo ci : ClassInfo.getAllClassInfo()) {
+if (ci.getTheClass() == newClass && ci != newClassInfo) {
+fail("Found multiple ClassInfo instances for class");
+}
+}
+}
+
+private void attemptDeadlock(final Class cls) throws Exception {
+for (int i = 0; i < THREAD_COUNT; i++) {
+Runnable runnable = new Runnable() {
+@Override
+public void run() {
+Class newClass = (cls == null) ?
createRandomClass() : cls;
+try {
+startLatch.await();
+} catch (InterruptedException ie) {
--- End diff --
can use `ThreadUtils.awaite()`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #491: GROOVY-8068: improper logging in groovy.sql.Sql
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/491#discussion_r99946115
--- Diff: subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java ---
@@ -578,17 +578,26 @@ public static Sql newInstance(Map<String, Object>
args) throws SQLException, Cla
Object url = sqlArgs.remove("url");
Connection connection;
+LOG.fine("url = " + url);
if (props != null) {
-System.err.println("url = " + url);
-System.err.println("props = " + props);
-connection = DriverManager.getConnection(url.toString(), new
Properties(props));
+Properties propsCopy = new Properties(props);
+connection = DriverManager.getConnection(url.toString(),
propsCopy);
+if (propsCopy.containsKey("password")) {
+// don't log the password
+propsCopy = new Properties(propsCopy);
--- End diff --
That makes sense, I missed that the copy was passed to the DriverManager.
Sorry for the noise.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #484: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/484 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #491: GROOVY-8068: improper logging in groovy.sql.Sql
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/491#discussion_r99844410
--- Diff: subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java ---
@@ -578,17 +578,26 @@ public static Sql newInstance(Map<String, Object>
args) throws SQLException, Cla
Object url = sqlArgs.remove("url");
Connection connection;
+LOG.fine("url = " + url);
if (props != null) {
-System.err.println("url = " + url);
-System.err.println("props = " + props);
-connection = DriverManager.getConnection(url.toString(), new
Properties(props));
+Properties propsCopy = new Properties(props);
+connection = DriverManager.getConnection(url.toString(),
propsCopy);
+if (propsCopy.containsKey("password")) {
+// don't log the password
+propsCopy = new Properties(propsCopy);
--- End diff --
I don't think this is needed since it's already been copied?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #490: GROOVY-8072: AstBrowser source view does not gener...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/490 GROOVY-8072: AstBrowser source view does not generate labels for stat⦠â¦ements You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8072 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/490.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #490 commit 757db0c9e69c237a6416e6ddff1d81a20818fd23 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-02-05T17:37:06Z GROOVY-8072: AstBrowser source view does not generate labels for statements --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #484: GROOVY-8067: Possible deadlock when creating new C...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/484 GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache While I have been able to replicate the deadlock between `GroovyClassValuePreJava7$Segment` and the `GlobalClassSet#add`, I have not directly observed one between the `modifiedExpandos` and the `GlobalClassSet`, but think that it would be good to isolate their reference processing too since both lock in their operations. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8067 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/484.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #484 commit 78f5aa0b5977919ba05dcad9fe8a7ee496abf2e8 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-01-29T18:26:43Z GROOVY-8067: test for demo only (DO NOT COMMIT) commit 58a27e7b1c437d436636658a5de9537fda5560d6 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-01-29T19:34:55Z GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #325: GROOVY-7646 - Classes generated by Eval() never co...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/325 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #477: AstBrowser fix showing Trait object initializers
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/477 AstBrowser fix showing Trait object initializers Fix to problem reported by @paulk-asert in PR #473 You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy4636-fix-traits Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/477.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #477 commit afe6390d13731fbb93a7a4ea64157400ce9ea913 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-01-14T21:51:06Z AstBrowser fix for Trait object initializers Fix to GROOVY-4636 that incorrectly handled adding statements to the AST node tree. commit f8cd15277db43d9082b3d3af86c3d78730d4ccc6 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-01-14T22:09:59Z minor cleanup and refactoring --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #475: GROOVY-5471: Add "indy" option to Groovy Console (...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/475 GROOVY-5471: Add "indy" option to Groovy Console (and AstBrowser) You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy5471 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/475.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #475 commit 8a8fd94a09491f24c692df5aa132de9c34bed70d Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2017-01-03T06:42:21Z GROOVY-5471: Add "indy" option to Groovy Console (and AstBrowser) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #473: GROOVY-4636: AST Browser does not show object init...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/473 GROOVY-4636: AST Browser does not show object initializer statements You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy4636 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/473.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #473 commit 6be1eefa1790134c9d352dd6518fe281826617c0 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2016-12-31T18:47:32Z GROOVY-4636: AST Browser does not show object initializer statements --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #462: GROOVY-6175: invoking Closure property like method...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/462 GROOVY-6175: invoking Closure property like method fails because of doCall Per the Closure class docs, to be able to use a Closure in short form, e.g., c(), in subclasses, you need to provide a doCall method with any signature you want to. This ensures that getMaximumNumberOfParameters() and getParameterTypes() will work too without any additional code. If no doCall method is provided a closure must be used in its long form, e.g., c.call(). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy GROOVY-6175 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/462.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #462 commit 76cffc9bab8825389258034cc0c231603ebe619e Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2016-11-22T03:54:10Z GROOVY-6175: invoking Closure property like method fails because of doCall/call asynchronity Per the Closure class docs, to be able to use a Closure in short form, e.g., c(), in subclasses, you need to provide a doCall method with any signature you want to. This ensures that getMaximumNumberOfParameters() and getParameterTypes() will work too without any additional code. If no doCall method is provided a closure must be used in its long form, e.g., c.call(). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/3c074dc2058d4c5115f172094385d9efd302f3ce#commitcomment-19444132 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #432: GROOVY-7909 Calling parents method from trait usin...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/432#discussion_r83546094
--- Diff:
src/main/org/codehaus/groovy/transform/trait/SuperCallTraitTransformer.java ---
@@ -36,13 +38,19 @@
import java.util.List;
+import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
+
/**
* This transformer is used to transform calls to
SomeTrait.super.foo() into the appropriate trait call.
*
* @author Cédric Champeau
* @since 2.3.0
*/
class SuperCallTraitTransformer extends ClassCodeExpressionTransformer {
+public static final String UNRESOLVED_HELPER_CLASS =
"UNRESOLVED_HELPER_CLASS";
--- End diff --
This is only used in the same package, I think package-private would be
better than public.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #438: GROOVY-7948: fix completion of static imports in g...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/438#discussion_r83096173
--- Diff:
subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy
---
@@ -87,51 +78,49 @@ class ImportsSyntaxCompletor implements
IdentifierCompletor {
return foundMatch
}
-private static final String STATIC_IMPORT_PATTERN = ~/^import static
([a-z0-9]+\.)+[A-Z][a-zA-Z0-9]*(\.(\*|[^.]+))?$/
+private static final String STATIC_IMPORT_PATTERN = ~/^static
([a-zA-Z_][a-zA-Z_0-9]*\.)+([a-zA-Z_][a-zA-Z_0-9]*|\*)$/
/**
* finds matching imported classes or static methods
- * @param prefix
- * @param importSpec
- * @param matches
- * @return
+ * @param importSpec an import statement without the leading 'import '
or trailing semicolon
+ * @return all names matching the importSpec
*/
-void collectImportedSymbols(final String importSpec, final
Collection matches) {
+SortedSet collectImportedSymbols(final String importSpec) {
--- End diff --
While I don't think `collectImportedSymbols` was meant to be called outside
this class there is a chance it could be since it's a public method. So to
avoid a breaking change and to have the chance to include this in the next 2.4
release (2.4.8) it would be necessary to keep the `void
collectImportedSymbols(String,Collection)` method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #432: GROOVY-7909 Calling parents method from trait usin...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/432#discussion_r82406821 --- Diff: src/test/groovy/bugs/Groovy7909Bug.groovy --- @@ -0,0 +1,57 @@ +package groovy.bugs --- End diff -- Could you add the license header to this file, should be able to grab it from another file in the same package. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #371: Serialization options for JsonOutput
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/371#discussion_r82402368
--- Diff: subprojects/groovy-json/src/main/java/groovy/json/JsonOutput.java
---
@@ -601,4 +280,966 @@ public String toString() {
}
}
+/**
+ * Creates a builder for various options that can be set to alter the
+ * generated JSON. After setting the options a call to
+ * {@link Options#createGenerator()} will return a fully configured
+ * {@link JsonOutput.Generator} object and the {@code toJson} methods
+ * can be used.
+ *
+ * @return a builder for building a JsonOutput.Generator
+ * with the specified options set.
+ * @since 2.5
+ */
+public static Options options() {
+return new Options();
+}
+
+/**
+ * A builder used to construct a {@link JsonOutput.Generator} instance
that allows
+ * control over the serialized JSON output. If you do not need to
customize the
+ * output it is recommended to use the static {@code
JsonOutput.toJson} methods.
+ *
+ *
+ * Example:
+ *
+ * def generator = groovy.json.JsonOutput.options()
+ * .excludeNulls()
+ * .dateFormat('')
+ * .excludeFieldsByName('bar', 'baz')
+ * .excludeFieldsByType(java.sql.Date)
+ * .createGenerator()
+ *
+ * def input = [foo: null, lastUpdated: Date.parse('-MM-dd',
'2014-10-24'),
+ * bar: 'foo', baz: 'foo', systemDate: new
java.sql.Date(new Date().getTime())]
+ *
+ * assert generator.toJson(input) == '{"lastUpdated":"2014"}'
+ *
+ *
+ * @since 2.5
+ */
+public static class Options {
+
+private boolean excludeNulls;
+
+private boolean disableUnicodeEscaping;
+
+private String dateFormat = JsonOutput.JSON_DATE_FORMAT;
+
+private Locale dateLocale = JsonOutput.JSON_DATE_FORMAT_LOCALE;
+
+private TimeZone timezone =
TimeZone.getTimeZone(JsonOutput.DEFAULT_TIMEZONE);
+
+private final Set converters = new
LinkedHashSet();
+
+private final Set excludedFieldNames = new
HashSet();
+
+private final Set<Class> excludedFieldTypes = new
HashSet<Class>();
+
+private Options() {}
+
+/**
+ * Do not serialize {@code null} values.
+ *
+ * @return a reference to this {@code Options} instance
+ */
+public Options excludeNulls() {
+excludeNulls = true;
+return this;
+}
+
+/**
+ * Disables the escaping of Unicode characters in JSON String
values.
+ *
+ * @return a reference to this {@code Options} instance
+ */
+public Options disableUnicodeEscaping() {
+disableUnicodeEscaping = true;
+return this;
+}
+
+/**
+ * Sets the date format that will be used to serialize {@code
Date} objects.
+ * This must be a valid pattern for {@link
java.text.SimpleDateFormat} and the
+ * date formatter will be constructed with the default locale of
{@link Locale#US}.
+ *
+ * @param format date format pattern used to serialize dates
+ * @return a reference to this {@code Options} instance
+ * @exception NullPointerException if the given pattern is null
+ * @exception IllegalArgumentException if the given pattern is
invalid
+ */
+public Options dateFormat(String format) {
+return dateFormat(format, JsonOutput.JSON_DATE_FORMAT_LOCALE);
+}
+
+/**
+ * Sets the date format that will be used to serialize {@code
Date} objects.
+ * This must be a valid pattern for {@link
java.text.SimpleDateFormat}.
+ *
+ * @param format date format pattern used to serialize dates
+ * @param locale the locale whose date format symbols will be used
+ * @return a reference to this {@code Options} instance
+ * @exception IllegalArgumentException if the given pattern is
invalid
+ */
+public Options dateFormat(String format, Locale locale) {
+// validate date format pattern
+new SimpleDateFormat(format, locale);
+dateFormat = format;
+dateLocale =
[GitHub] groovy pull request #371: Serialization options for JsonOutput
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/371#discussion_r8233
--- Diff: subprojects/groovy-json/src/main/java/groovy/json/JsonOutput.java
---
@@ -492,7 +171,7 @@ private static void writeIterator(Iterator iterator,
CharBuf buffer) {
*/
public static String prettyPrint(String jsonPayload) {
int indentSize = 0;
-// Just a guess that the pretty view will take 20 percent more
than original.
+// Just a guess that the pretty view will take a 20 percent more
than original.
--- End diff --
Thanks for pointing that out I'll remove the `a`. I missed that when
fixing up the merge conflict caused by commit 7e8211fa314526ce.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #440: Groovy-7961 : NoSuchElementException
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/440#discussion_r82316120
--- Diff: src/main/groovy/lang/ObjectRange.java ---
@@ -450,6 +451,9 @@ public Comparable next() {
value = peek();
nextFetched = true;
}
+if (value == null) {
+throw new NoSuchElementException();
+}
--- End diff --
I think these first 2 `if` statements could be replaced by the following
and would make it cleaner:
```
if (!hasNext()) {
throw new NoSuchElementException();
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #429: GROOVY-7946 - allow writable values for StreamingJ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/429#discussion_r80391169 --- Diff: subprojects/groovy-json/src/main/java/groovy/json/StreamingJsonBuilder.java --- @@ -651,6 +655,18 @@ public void call(String name, JsonOutput.JsonUnescaped json) throws IOException writer.write(json.toString()); } +/** + * Writes the given Writable as the value of the given attribute name + * + * @param name The attribute name The attribute name + * @param json The value The writable --- End diff -- descriptions for the params seems to be repeated twice. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #427: Compatibility in 2.4.x with Gradle's classloader c...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/427 Compatibility in 2.4.x with Gradle's classloader cleanup Gradle has a cleanup mechanism in place that reflectively accesses the ClassInfo.klazz field. This change prevents an exception being thrown by the cleanup method. This workaround also means that the cleanup strategy will no longer work, the fix to replace klazz with a WeakReference should eliminate the need for the explicit cleanup. For more details, see dev mailing list thread: https://mail-archives.apache.org/mod_mbox/incubator-groovy-dev/201609.mbox/%3CCAHPL-JkQ%2BU8PfaxyVhtE%3DvGV%2BsXXmCzxOsCR8UbBww6P2vskPg%40mail.gmail.com%3E You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy ClassInfoCompat Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/427.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #427 commit 81dfa648541f00a33d25bc0084ec240b88c3f321 Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2016-09-19T18:48:19Z Compatibility in 2.4.x with Gradle's classloader cleanup Gradle has a cleanup mechanism in place that reflectively accesses the ClassInfo.klazz field. This change prevents an exception being thrown by the cleanup method. This workaround also means that the cleanup strategy will no longer work, the fix to replace klazz with a WeakReference should eliminate the need for the explicit cleanup. For more details, see dev mailing list thread: https://mail-archives.apache.org/mod_mbox/incubator-groovy-dev/201609.mbox/%3CCAHPL-JkQ%2BU8PfaxyVhtE%3DvGV%2BsXXmCzxOsCR8UbBww6P2vskPg%40mail.gmail.com%3E --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #424: GROOVY-7933: fix method selection for boolean and ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/424 GROOVY-7933: fix method selection for boolean and char primitives boolean and char were not factored into the primitive distance tables which would cause methods with Object to be selected in preference to those with boolean/char and their respective Wrapper. Inserted `boolean` and `char` (and their wrappers) in the distance table and kept all distances the same for the existing classes in the table (adding boolean and char as the farthest dist from the existing classes). Had to fix one test because `HashCodeHelper` has overloaded methods and prior to this change it was selecting the method with `Object` instead of `boolean`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy GROOVY-7933 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/424.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #424 commit 156d3f72db9e80db6f62bdc3829f2d3767c9544e Author: John Wagenleitner <jwagenleit...@apache.org> Date: 2016-09-18T05:41:06Z GROOVY-7933: fix method selection for boolean and char primitives boolean and char were not factored into the primitive distance tables which would cause methods with Object to be selected in preference to those with boolean/char and their respective Wrapper. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #422: GROOVY-7922: Static type checking not strict enoug...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/422#discussion_r79297750
--- Diff:
src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java ---
@@ -1157,6 +1142,45 @@ private static ClassNode makeRawType(final ClassNode
receiver) {
return result;
}
+private static void removeMethodInSuperInterface(List
toBeRemoved, MethodNode one, MethodNode two) {
+ClassNode oneDC=one.getDeclaringClass();
+ClassNode twoDC=two.getDeclaringClass();
+if(oneDC.implementsInterface(twoDC)){
+toBeRemoved.add(two);
+}else{
+toBeRemoved.add(one);
+}
+}
+
+private static boolean areEquivalentInterfaceMethods(MethodNode one,
MethodNode two, Parameter[] onePars, Parameter[] twoPars) {
--- End diff --
I think it would simplify things if this just took 2 MethodNode's, the
parameters can be gotten from the nodes so no need to pass them in I think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #422: GROOVY-7922: Static type checking not strict enoug...
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/422#discussion_r79297788
--- Diff:
src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java ---
@@ -1116,39 +1117,23 @@ private static ClassNode makeRawType(final
ClassNode receiver) {
for (int j=i+1;j<list.size();j++) {
MethodNode two = list.get(j);
if (toBeRemoved.contains(two)) continue;
-if (one.getName().equals(two.getName()) &&
one.getDeclaringClass()==two.getDeclaringClass()) {
-Parameter[] onePars = one.getParameters();
-Parameter[] twoPars = two.getParameters();
-if (onePars.length == twoPars.length) {
-boolean sameTypes = true;
-for (int k = 0; k < onePars.length; k++) {
-Parameter onePar = onePars[k];
-Parameter twoPar = twoPars[k];
-if
(!onePar.getType().equals(twoPar.getType())) {
-sameTypes = false;
-break;
-}
-}
-if (sameTypes) {
-ClassNode oneRT = one.getReturnType();
-ClassNode twoRT = two.getReturnType();
-if (oneRT.isDerivedFrom(twoRT) ||
oneRT.implementsInterface(twoRT)) {
-toBeRemoved.add(two);
-} else if (twoRT.isDerivedFrom(oneRT) ||
twoRT.implementsInterface(oneRT)) {
-toBeRemoved.add(one);
-}
+Parameter[] onePars = one.getParameters();
+Parameter[] twoPars = two.getParameters();
+if (onePars.length == twoPars.length) {
+if (areOverloadMethodsInSameClass(one,two)) {
+if (ParameterUtils.parametersEqual(onePars,
twoPars)){
--- End diff --
If the `Parameter[]` parameters are removed from
`areEquivalentInterfaceMethods` method I think these arguments can be replaced
with `one.getParameters(), two.getParameters()` and then the local variables
wouldn't be needed. Just a suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
