Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] InMemoryJavaFileManager.uriForJavaFileObject() breaks when supplied with a Locations.ModuleLocationHandler #335

Open
CC007 opened this issue Oct 2, 2022 · 4 comments
Labels
P3 type=defect Bug, not working as expected

Comments

@CC007
Copy link

CC007 commented Oct 2, 2022

In InMemoryJavaFileManager the method uriForJavaFileObject() is defined as such:

private static URI uriForJavaFileObject(Location location, String className, Kind kind) {
return URI.create(
    "mem:///" + location.getName() + '/' + className.replace('.', '/') + kind.extension);
}

EXPECTED:
This method works for any Location object.

ACTUAL:

This breaks when using modules.

When the JavacTask runs validate() on the arguments in JavacTaskImpl.prepareCompiler(), if a module has been found, the CLASS_OUTPUT location will be determined by asking the JavaFileManager for the location for that module

fm.getLocationForModule(StandardLocation.CLASS_OUTPUT, moduleName)

This will lead to Locations.OutputLocationHandler.getLocationForModule(moduleName) being called, which returns a Location of type Locations.ModuleLocationHandler with the name set to location.getName() + "[" + name + "]" eg. "CLASS_OUTPUT[foo]" if the module name is "foo".

@Override
Location getLocationForModule(String name) {
    if (moduleTable == null) {
        moduleTable = new ModuleTable();
    }
    ModuleLocationHandler l = moduleTable.get(name);
    if (l == null) {
        Path out = outputDir.resolve(name);
        l = new ModuleLocationHandler(this, location.getName() + "[" + name + "]",
                name, Collections.singletonList(out), true);
        moduleTable.add(l);
    }
    return l;
}

The validate() method will then call getJavaFileForInput() using this location, which calls the InMemoryJavaFileManager.uriForJavaFileObject() method.

Due to location.getName returning "CLASS_OUTPUT[foo]", URL.create will throw the following exception:

java.lang.IllegalArgumentException: Illegal character in path

It gives index 19, which is the square opening bracket [ in mem:///CLASS_OUTPUT[test]/....

This makes it not possible to directly use location.getName() in the uriForJavaFileObject() method for any given Location object, without encoding this name in some form, before passing it along to URL.create()

SOLUTION:
Encode the location name in some way, so that illegal URI characters won't be passed along to the URL.create() method.

VERSIONS USED:
compile-testing: 0.19
JDK: AZUL-17 version 17.0.4.1

@chaoren chaoren added type=defect Bug, not working as expected P2 labels Oct 3, 2022
@chaoren
Copy link
Member

chaoren commented Oct 3, 2022

Thanks for digging into the details. Could you please provide some example code (e.g., a javac().compile(...) call) that demonstrates the issue?

@CC007
Copy link
Author

CC007 commented Oct 4, 2022

You can find the test here: https://github.com/CodeQualIT/NullValidation/blob/master/NullValidationProcessor/src/test/java/nl/cqit/validation/nullsafe/processor/internal/NotNullCheckerTest.java

I also had a hard time configuring the modules directly through methods, so I supplied them as javac execution options. These options seem to work if used on the command line with javac directly (though it throws an exception during execution of the annotation processor, as it isn't fully implemented yet).

So expected behavior in that test: throws java.lang.IndexOutOfBoundsException: Index: 0, Size: 0,
Actual behavior: throws java.lang.RuntimeException: java.lang.IllegalArgumentException: Illegal character in path at index 19: mem:///CLASS_OUTPUT[test]/module-info.class

@CC007
Copy link
Author

CC007 commented Oct 4, 2022

Updated the code so that it now gives no error from the commandline, but it still gives the IllegalArgumentException when the test is run.

(I am using this command from the NullValidationProcessor\src\test\resources folder:

javac -d classes --module-path ..\..\..\..\NullValidationAnnotations\target\null-safe-annotations-1.0-SNAPSHOT.jar --module-source-path .\ --module test -processorpath '..\..\..\target\null-safe-processor-1.0-SNAPSHOT.jar;..\..\..\..\NullValidationAnnotations\target\null-safe-annotations-1.0-SNAPSHOT.jar'

@chaoren chaoren added P3 and removed P2 labels Oct 5, 2022
@chaoren
Copy link
Member

chaoren commented Oct 5, 2022

It seems modules support in compile-testing is just completely non-existent at the moment. I don't think it makes sense to fix this particular issue until we decide to add support for compiling modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 type=defect Bug, not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants