search

INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.76k stars 352 forks source link

[Bug] Ambiguous package name detected #4633

Open darius-sas opened 2 years ago

darius-sas commented 2 years ago

Describe the bug An exception is raised when analysing two files within the same package but in two different folders in the file system, like for example source code files and test files. This only seems to happen when the project under analysis uses modules.

It seems that this error was "fixed" by another issue (#4051) but it seems that's not the case, as I still get the error even though I'm using Spoon 10.0.0.

To Reproduce I encountered the error while analysing JetUML, so I'm going to us that to describe how to reproduce the error.

First clone the repo:

git clone --depth 1 https://github.com/prmr/JetUML.git /tmp/JetUML

Then run the following test

@Test
    void testAmbiguousName() {
        Launcher launcher = new Launcher();
        Environment environment = launcher.getEnvironment();
        environment.setEncoding(StandardCharsets.UTF_8);
        environment.setComplianceLevel(16);
        environment.setIgnoreDuplicateDeclarations(true);
        environment.setCommentEnabled(false);
        environment.disableConsistencyChecks();
        environment.setNoClasspath(true);
        environment.setShouldCompile(false);

        launcher.addInputResource("/tmp/JetUML/src/module-info.java"); // Commenting this line of code out makes the test pass.
        launcher.addInputResource("/tmp/JetUML/src/ca/mcgill/cs/jetuml/viewers/nodes/AbstractNodeViewer.java");
        launcher.addInputResource("/tmp/JetUML/test/ca/mcgill/cs/jetuml/viewers/nodes/TestUseCaseNodeViewer.java");
        Assertions.assertDoesNotThrow(() -> launcher.buildModel());
    }

Output:

spoon.SpoonException: Ambiguous package name detected. If you believe the code you analyzed is correct, please file an issue and reference https://github.com/INRIA/spoon/issues/4051. Error details: Found 2 non-empty packages with name 'ca.mcgill.cs.jetuml.viewers.nodes'
    at spoon.reflect.factory.PackageFactory.get(PackageFactory.java:192)
    at spoon.support.reflect.reference.CtPackageReferenceImpl.getDeclaration(CtPackageReferenceImpl.java:24)
    at spoon.support.reflect.declaration.CtCompilationUnitImpl.getDeclaredPackage(CtCompilationUnitImpl.java:230)
    at spoon.support.compiler.jdt.ReferenceBuilder.setPackageOrDeclaringType(ReferenceBuilder.java:1188)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:705)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:638)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:517)
    at spoon.support.compiler.jdt.ReferenceBuilder.getExecutableReference(ReferenceBuilder.java:455)
    at spoon.support.compiler.jdt.JDTTreeBuilder.visit(JDTTreeBuilder.java:879)
    at org.eclipse.jdt.internal.compiler.ast.AllocationExpression.traverse(AllocationExpression.java:716)
    at org.eclipse.jdt.internal.compiler.ast.FieldDeclaration.traverse(FieldDeclaration.java:381)
    at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1692)
    at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:827)
    at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:788)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.traverseUnitDeclaration(JDTBasedSpoonCompiler.java:480)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.lambda$buildModel$0(JDTBasedSpoonCompiler.java:437)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.forEachCompilationUnit(JDTBasedSpoonCompiler.java:464)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildModel(JDTBasedSpoonCompiler.java:435)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildUnitsAndModel(JDTBasedSpoonCompiler.java:372)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildSources(JDTBasedSpoonCompiler.java:335)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:116)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:99)
    at spoon.Launcher.buildModel(Launcher.java:782)spoon.SpoonException: Ambiguous package name detected. If you believe the code you analyzed is correct, please file an issue and reference https://github.com/INRIA/spoon/issues/4051. Error details: Found 2 non-empty packages with name 'ca.mcgill.cs.jetuml.viewers.nodes'
    at spoon.reflect.factory.PackageFactory.get(PackageFactory.java:192)
    at spoon.support.reflect.reference.CtPackageReferenceImpl.getDeclaration(CtPackageReferenceImpl.java:24)
    at spoon.support.reflect.declaration.CtCompilationUnitImpl.getDeclaredPackage(CtCompilationUnitImpl.java:230)
    at spoon.support.compiler.jdt.ReferenceBuilder.setPackageOrDeclaringType(ReferenceBuilder.java:1188)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:705)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:638)
    at spoon.support.compiler.jdt.ReferenceBuilder.getTypeReference(ReferenceBuilder.java:517)
    at spoon.support.compiler.jdt.ReferenceBuilder.getExecutableReference(ReferenceBuilder.java:455)
    at spoon.support.compiler.jdt.JDTTreeBuilder.visit(JDTTreeBuilder.java:879)
    at org.eclipse.jdt.internal.compiler.ast.AllocationExpression.traverse(AllocationExpression.java:716)
    at org.eclipse.jdt.internal.compiler.ast.FieldDeclaration.traverse(FieldDeclaration.java:381)
    at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1692)
    at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:827)
    at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:788)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.traverseUnitDeclaration(JDTBasedSpoonCompiler.java:480)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.lambda$buildModel$0(JDTBasedSpoonCompiler.java:437)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.forEachCompilationUnit(JDTBasedSpoonCompiler.java:464)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildModel(JDTBasedSpoonCompiler.java:435)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildUnitsAndModel(JDTBasedSpoonCompiler.java:372)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildSources(JDTBasedSpoonCompiler.java:335)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:116)
    at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:99)
    at spoon.Launcher.buildModel(Launcher.java:782)

Operating system, JDK and Spoon version used

Any ideas on how to deal with the issue?

Thank you.

slarse commented 2 years ago

@darius-sas Thanks for the bug report, we'll look at this.

@I-Al-Istannen Good thing we put that elaborate error message there. If I recall correctly, we raise here because we assume that a package can only be exported from one single module. I think we also equated "one module" to "one root directory", but here there seems to be two root directories for the module: tests and src. I'm frankly not certain how this kind of setup fits in to the module system.

I-Al-Istannen commented 2 years ago

@slarse That was a long range snipe, I shouldn't have put the issue number in the message :P Nobody would have known who to blame!

I believe that src and test can not share any packages, as a package can only be exported once for a single classloader. However, it seems that surefire hacks its way around that using --patch-module to effectively add them to the main bundle. This is what makes the whole thing work in practice.

I am not sure how we want to handle this, as it working in the first place seems to be a hack of the build tool. If we assume that no single type is defined twice (which is a much weaker assumption), we could graciously handle this case - but then we need to walk down the whole package tree in all modules again. I don't think we found a nice solution doing that in the original issue.

We could also follow surefire's footsteps and collapse modules as we deem fit, but that sounds like a scary thing to do for an analysis tool.

darius-sas commented 2 years ago

Thank you for the quick reply, @slarse and @I-Al-Istannen.

I'm no expert on the matter, so this might not make any sense.

If I understood correctly the problem is that the test package is not within the same module as the src one, but they define the same package. Then why don't treat these as two separate packages that live in two different modules? One module is the one defined in src and the other is the unnamed module. This would be consistent with how the code is organized at the file system level, though it might not make sense logically.

I-Al-Istannen commented 2 years ago

A bit of a wall of text, sorry about that...

The problem with that is that looking up classes by qualified name currently does not include the module name in the qualified name. So you could now find two different classes for a qualified name with no way to know what to prefer. As spoon explicitly models packages and finds classes by walking down the tree, this also applies to everything in a subpackage of an ambiguous package, not just actually duplicated classes. In essence this would require package (and maybe type) lookup to return a list instead of an optional. Making this work without would probably entail replacing the package-tree visiting logic of spoon with something that simultaneously explores all possible paths in all modules at every step and prays it finds a single answer at the end. If a package itself is looked up though, we are completely out of luck as the package lookup API does not know which module you want and packages can be duplicated in multiple modules. This would be easier if the lookup APIs always included a module name, but somehow the original spoon developers did not have the foresight 15 years ago and I don't think I can blame them.

We could require the qualified names to include the module at the start, but that would be problematic for code not using modules (though we could omit them there) and would break about anything already written using qualified names. It also differs from the JLS definition of fully qualified, which does not include the module name as far as I can tell.

We could allow the qualified name to start with the module if the code uses modules, but that would still introduce the ambiguity above. It would also require reworking quite a few places where spoon internally bases lookups on qualified names.

I am honestly not sure what the a good solution would be here. We could offer something similar to --patch-module or add an option to automatically collapse potentially dangerous modules. We are in the unlucky position to statically analyze code that is fixed up by the build tool. I'd assume surefire fixing stuff is common enough that we still need to find a suitable solution though.

slarse commented 2 years ago

:point_up: That.

I'd assume surefire fixing stuff is common enough that we still need to find a suitable solution though.

This is unfortunately what I'm thinking as well.

We could offer something similar to --patch-module or add an option to automatically collapse potentially dangerous modules

Something like that. It would have to be opt-in. I think there may be unseen complications here as well, for example I think printing will also collapse everything into a single root unless we are very firm in telling Spoon not to.

I'm inclined to label this ticket as a feature request rather than a bug.

@I-Al-Istannen Regardless, this is not high priority, let's get through the type adaption first and then return to this.

honghao12 commented 8 months ago

This would be easier if the lookup APIs always included a module name, but somehow the original spoon developers did not have the foresight 15 years ago and I don't think I can blame them.

Are there any plans to add module names to the api to solve this problem