-
Notifications
You must be signed in to change notification settings - Fork 139
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
"Missing System Library" exception in ASTParser #3298
Comments
See #3047 for context. |
This check should probably only be done for proper Java projects? |
@stephan-herrmann can you take a look? |
This issue can still be reproduced.
@ptziegler can you propose such PR please? |
There is a lot of stuff on my todo list right now, but think an initial draft is done fairly quickly. I'll try to come up with something during the weekend. |
After a quick glance, I don't think adding an additional check in the AST parser is not the correct solution. Instead, I believe that the project being converted to a JavaProject is the real problem. From the documentation:
This is not the case, unless a project has the Java nature. And if the |
Which code creates the |
I believe the JavaProject is created by the CompilationUnitDocumentProvider when the Java editor is openend.. More specifically, by the JavaModelManager: eclipse.jdt.core/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java Lines 964 to 981 in 2c5a057
However, I get the impression that there is no way around this, as the ICompilationUnit is created by an IPackageFragment, which in returns requires the JavaProject. 😦 |
I looked deeper into this issue and I believe the underlying problem is that the Java editor is always opened with the eclipse.jdt.core/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/ASTParser.java Lines 270 to 272 in 7ddead3
If I add a check that clears this flag if the Java project doesn't have the Java nature, this exception isn't thrown. Meaning I now have two options: I can either fix it in the ASTParser class, where it would also handle all other combinations I haven't thought of, or in the UI classes for this specific use case. Given that I'm not overly familiar with either classes, what would be the preferable approach? I'm personally leaning towards the latter option, because I think it's wrong to do a type binding when there are no types to bind with. Depending on that, I would push a PR either to this repository or to JDT UI. |
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
I've created #3560 as a fix, after I noticed that this actively breaks our application, because we parse source code that isn't in a Java project. I therefore believe it's not sufficient to only fix this for the Java editor, as there are many more instances where this problem can and will occur. |
@ptziegler You might consider adding such a test using ASTParser in non Java project to make sure it doesn't regress again in the future. |
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
Isn't this exactly what I'm doing with this test case? public void testGH3298() throws Exception {
Hashtable<String, String> options = JavaCore.getDefaultOptions();
options.put(JavaCore.COMPILER_COMPLIANCE, JavaCore.VERSION_9);
options.put(JavaCore.COMPILER_SOURCE, JavaCore.VERSION_9);
options.put(JavaCore.COMPILER_CODEGEN_TARGET_PLATFORM, JavaCore.VERSION_9);
IProject project = createProject("P");
IJavaProject javaProject = JavaCore.create(project);
try {
ASTParser parser = ASTParser.newParser(AST.getJLSLatest());
parser.setProject(javaProject);
parser.setCompilerOptions(options);
parser.setSource("public class Test{}".toCharArray());
parser.setResolveBindings(true);
parser.setKind(ASTParser.K_COMPILATION_UNIT);
assertNotNull(parser.createAST(null));
addJavaNature("P");
parser.setProject(javaProject);
parser.setCompilerOptions(options);
parser.setSource("public class Test{}".toCharArray());
parser.setResolveBindings(true);
parser.setKind(ASTParser.K_COMPILATION_UNIT);
assertThrows(IllegalStateException.class, () -> parser.createAST(null));
parser.setProject(javaProject);
parser.setCompilerOptions(options);
parser.setSource("public class Test{}".toCharArray());
parser.setResolveBindings(false);
parser.setKind(ASTParser.K_COMPILATION_UNIT);
assertNotNull(parser.createAST(null));
} finally {
deleteProject("P");
}
} |
As a side note: The |
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
What I've been thinking of a test without IJavaProject javaProject = JavaCore.create(project); and parser.setProject(javaProject); lines to ensure that ASTParser works in "plain" projects. |
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
Ah, thanks for the clarification I've updated the test case to avoid using the |
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
The validation of the project classpath introduced with 74d062b always fails, if the project doesn't have a Java nature (and thus usually no .classpath file). This causes issues when e.g. trying to open a source file with the Java editor or simply when trying to use the AST parser outside of a Java project. With this change, the check becomes more conservative and only fails, if both the Java nature exists and no system library is configured. Note that one can avoid this exception by disabling the binding resolution. But given that this requires changes in user code, it is more prudent to only fail in cases where this is the desired behavior. Resolves eclipse-jdt#3298
This seems to be a regression introduced by 74d062b
To reproduce: Make sure compiler compliance is set to > 1.8 and open a Java file from a project without Java nature.
Stack Trace:
The text was updated successfully, but these errors were encountered: