javaparser
javaparser copied to clipboard
javaparser
Multiple providers for single SPI in module-info.java is not treated as an error
The following module-info.java source is invalid
module com.example {
exports com.example;
provides com.example.Service with com.example.Provider1;
provides com.example.Service with com.example.Provider2;
}
But v3.25.1 does not flag it as an error. Affects https://github.com/moditect/moditect/issues/173
The correct syntax is
module com.example {
exports com.example;
provides com.example.Service with com.example.Provider1, com.example.Provider2;
}
ModiTect fails to detect this problem, generates a module-info.class file that causes the following error when loaded
Error occurred during initialization of boot layer
java.lang.module.FindException: Error reading module: target/generated-test-modules/example-providers.jar
Caused by: java.lang.module.InvalidModuleDescriptorException: Providers of service com.example.Service already declared
Sure. How would you like me to do it? Should I attach a patch with a test file or a PR that breaks the build when merged?
You have declared a problem in javaparser, it would only be necessary to provide the java code that illustrates the problem. However I have the feeling that it is a misuse of the API provided that led to inadequate content. Javaparser provides an API that allows you to modify the AST of java code, but it does not aim to ensure the consistency of what is produced using the API.
Here's how ModiTect consumes javaparser to generate a ModuleInfo
https://github.com/moditect/moditect/blob/b75169187d7f35dbde2a85d6a196f88d92a2e1a3/core/src/main/java/org/moditect/internal/compiler/ModuleInfoCompiler.java#L67-L72
moduleInfoSource is just plain text, exactly as it appears in the first comment. I've just pushed the failing testcase to a branch in the ModiTect repo which exemplifies the usage.
- git clone https://github.com/moditect/moditect.git
- checkout the java-parser-3942 branch
- mvn verify
This results in a bad module-info.class due to invalid provider declarations.
Thank you for your effort but what is expected is a simple unit test that reproduces the problem (probably about ten lines of code). Be aware, however, that in the same way that the java code analyzed by javaparser must be compiled without error, it is the same for a module description. If you try to parse with JP an erroneous module declaration, the results may be unexpected.
Understood. That's why I asked how would you like me to send the failing test case:
- as a patch file attached to this issue
- as a PR
Since we are talking about unit tests you can copy/paste the test code in the issue. That way I could quickly reproduce your test case without having to install anything.
Here we go
/*
* Copyright (C) 2013-2023 The JavaParser Team.
*
* This file is part of JavaParser.
*
* JavaParser can be used either under the terms of
* a) the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
* b) the terms of the Apache License
*
* You should have received a copy of both licenses in LICENCE.LGPL and
* LICENCE.APACHE. Please refer to those files for details.
*
* JavaParser is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*/
package com.github.javaparser;
import com.github.javaparser.ast.CompilationUnit;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
public class Issue3942Test {
@Test()
void test() {
String code = "module com.example {\n" +
" exports com.example;\n" +
" provides com.example.Service with com.example.Provider1;\n" +
" provides com.example.Service with com.example.Provider2;\n" +
"}";
Assertions.assertThrows(ParseProblemException.class, () -> {
CompilationUnit cu = StaticJavaParser.parse(code);
});
}
}
What is the expected result? A ParseProblemException? I don't think there is a syntax error.
The expected result is a ParseProblemException or any other exception that the parser may throw when inputs are invalid.
If there is no syntax error which seems to be the case, there is no reason to have a parsing error.
If I dared the comparison I would say that you can perfectly parse code that contains bugs without JP throwing an exception.
But that is what I've been trying to say, the module definition is invalid
module com.example {
exports com.example;
provides com.example.Service with com.example.Provider1;
provides com.example.Service with com.example.Provider2;
}
There is no way an IDE or the compiler will let you produce bytecode with this definition, yet Javaparser has no problem parsing it.
If I dared the comparison I would say that you can perfectly parse code that contains bugs without JP throwing an exception.
And that would be a bug in javaparser IF JP's goal is to accurately handle all of the Java Language syntax correctly.
I'm not an expert in defining modules in java, but if I understand your comments it's that the syntax is incorrect. What do you think the correct syntax would be?
The correct syntax is
module com.example {
exports com.example;
provides com.example.Service with com.example.Provider1, com.example.Provider2;
}
All implementations of the same provider must be grouped.
I'm looking the module declaration on JLS https://docs.oracle.com/javase/specs/jls/se11/html/jls-7.html#jls-ModuleDeclaration and I don't see what is syntactically incorrect in your example.
ModuleDirective:
requires {RequiresModifier} ModuleName ;
exports PackageName [to ModuleName {, ModuleName}] ;
opens PackageName [to ModuleName {, ModuleName}] ;
uses TypeName ;
provides TypeName with TypeName {, TypeName} ;
https://docs.oracle.com/javase/specs/jls/se11/html/jls-7.html#jls-7.7.4
It is a compile-time error if more than one provides directive in a module declaration specifies the same service.
It is a compile-time error if the with clause of a given provides directive specifies the same service provider more than once.
I was just reading this part. But the prerequisite for using JP is that the code must compile otherwise you may have unexpected behavior. What I understand from this part of the spec is that there can be multiple "requires" directives but they don't have to specify the same module name. So a module declaration can specify several "requires" directives and the fact that they relate to the same module name is considered a compilation error. So there is no problem on JP. It reacts as it should or as the designers of the project wanted it to react.
I'm not talking about requires but provides, unless you meant to write the latter but typed the former.
I was just reading this part. But the prerequisite for using JP is that the code must compile otherwise you may have unexpected behavior.
Fair. Perhaps I missed this fact. I suppose that requirement is found in the documentation, right?
Mind that if the code must compile otherwise you may have unexpected behavior likely means JP should never throw parse exceptions unless it does not support a new syntax construct added by a much recent version of the Java language.
I don't know but what is certain is that it is a logical prerequisite because JP is not a compiler. It is therefore necessary that the inputs are at least correct to be able to build an AST.
The whole point of the ModiTect tools is to inject a module-info.class into a JAR given a textual representation of a module definition. This textual representation is assumed to be correct before sending it to JP for parsing. Next, JP also assumes the input to be valid and creates the AST which by now has issues.
ModiTect takes textual input and generates a class file assuming JP and every other check in between went OK. It does not take the input, compile it, to later pass it to JP.
I believe the reason to pick JP in the first case was the expectation that JP would be able to handle proper inputs (as it does) as well as flagging bad input as errors. It seems that expectation is incorrect and thus an additional mechanism must be put in place before invoking JP to ensure that inputs are correct.
To be completely precise, there are validations which are carried out after the parsing which make it possible to implement rules which could not be facilitated with Javacc. This is a part that I do not know but I will look if it is consistent to implement this rule at this level.
Although it seems possible to integrate this type of validation after the creation of the AST, I do not think that it is the intention of the designers of this project because it would give rise to a large number of validation rules to check that the input code can be compiled. I will leave this outcome open but I do not think it is taken care of quickly.