This is an automated email from the ASF dual-hosted git repository. sunlan pushed a commit to branch GROOVY_3_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push: new c005209 GROOVY-9666: ConcurrentModificationException when processing ModuleNode imports (closes #1332) c005209 is described below commit c005209e83d8fbd3c622e3547611c74edcee657b Author: Paul King <pa...@asert.com.au> AuthorDate: Fri Jul 31 15:04:16 2020 +1000 GROOVY-9666: ConcurrentModificationException when processing ModuleNode imports (closes #1332) backwards compatibility trumps potential increased use of immutability (cherry picked from commit c7507b8a8cffe2aaa112eaef670ebc92b85dae48) --- .../java/org/codehaus/groovy/ast/ModuleNode.java | 25 ++++++++-- src/test/groovy/bugs/Groovy9666.groovy | 57 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java index 4cc6870..70ae0d6 100644 --- a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java +++ b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java @@ -91,24 +91,39 @@ public class ModuleNode extends ASTNode implements Opcodes { return /*Collections.unmodifiableList(*/classes/*)*/; // modified by MacroClassTransform } + /** + * @return the module's methods + */ public List<MethodNode> getMethods() { - return Collections.unmodifiableList(methods); + return methods; } + /** + * @return a copy of the module's imports + */ public List<ImportNode> getImports() { - return Collections.unmodifiableList(imports); + return new ArrayList<>(imports); } + /** + * @return the module's star imports + */ public List<ImportNode> getStarImports() { - return Collections.unmodifiableList(starImports); + return starImports; } + /** + * @return the module's static imports + */ public Map<String, ImportNode> getStaticImports() { - return Collections.unmodifiableMap(staticImports); + return staticImports; } + /** + * @return the module's static star imports + */ public Map<String, ImportNode> getStaticStarImports() { - return Collections.unmodifiableMap(staticStarImports); + return staticStarImports; } /** diff --git a/src/test/groovy/bugs/Groovy9666.groovy b/src/test/groovy/bugs/Groovy9666.groovy new file mode 100644 index 0000000..23714a1 --- /dev/null +++ b/src/test/groovy/bugs/Groovy9666.groovy @@ -0,0 +1,57 @@ +/* + * 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.bugs + +import org.codehaus.groovy.ast.ClassHelper +import org.codehaus.groovy.ast.ModuleNode +import org.codehaus.groovy.control.SourceUnit +import org.junit.Test + +final class Groovy9666 { + @Test + void testCanIterateImportsWhileAdding() { + def mn = new ModuleNode((SourceUnit)null) + + mn.addImport('Integer', ClassHelper.Integer_TYPE) + assert mn.imports.size() == 1 + for (importNode in mn.imports) { + mn.addImport('Natural', ClassHelper.Integer_TYPE) + } + assert mn.imports.size() == 2 + } + + @Test + void testSimulateImportCaseChangingTransform() { + def mn = new ModuleNode((SourceUnit)null) + + mn.addStarImport('foo.bar') + assert mn.starImports.size() == 1 + assert mn.starImports*.text == ['import foo.bar*'] + + // simulate xform that manipulates imports as per some DSL context + def copy = mn.starImports.clone() + mn.starImports.clear() + for (starImport in copy) { + mn.addStarImport(starImport.packageName.toUpperCase()) + } + + assert mn.starImports.size() == 1 + assert mn.starImports*.text == ['import FOO.BAR*'] + } +}