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

[java] Unable to parse some Java9+ resource references #4912

Closed
jsotuyod opened this issue Apr 3, 2024 · 2 comments · Fixed by #4934
Closed

[java] Unable to parse some Java9+ resource references #4912

jsotuyod opened this issue Apr 3, 2024 · 2 comments · Fixed by #4934
Labels
a:bug PMD crashes or fails to analyse a file. in:grammar About the grammar of a lexer or parser, eg, a parse/lex exception
Milestone

Comments

@jsotuyod
Copy link
Member

jsotuyod commented Apr 3, 2024

Affects PMD Version:
7.0.0

Description:

Since Java 9, PMD has allowed for concise resources (ie: not variables declared within the try-with-resources), but some constructs are currently rejected by the parser even if valid Java code.

Code Sample demonstrating the issue:

class Foo implements AutoCloseable {
      void testThis() throws Exception {
          try (this) { // unsupported
          }
      }

     void testQualifiedThis() throws Exception {
          try (Foo.this) { // unsupported
          }
      }

     void testArrayAccess(AutoCloseable[] c) throws Exception {
          try (c[0]) { // unsupported
          }
      }

      @Override
      public void close() throws Exception {
      }
}

Probably others too…

Running PMD through: Any

Probably, the saner fix would be to simply remove this check altogether, and continue under the premise that the code compiles…

{
Node top = jjtree.peekNode();
if (!(top instanceof ASTVariableAccess || top instanceof ASTFieldAccess))
throwParseException("Expected a variable access, but was a " + top.getXPathNodeName());
}
{}

@jsotuyod jsotuyod added a:bug PMD crashes or fails to analyse a file. in:grammar About the grammar of a lexer or parser, eg, a parse/lex exception labels Apr 3, 2024
@jsotuyod jsotuyod added this to the 7.0.1 milestone Apr 3, 2024
@oowekyala
Copy link
Member

oowekyala commented Apr 4, 2024

Interesting... We would need to make this method nullable and adapt the callers: https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTResource.java#L52

Especially EmptyControlStatementRule needs new tests to disallow these constructs. Writing

try (this) {}

is weird and should be this.close(). I opened #4930.

@oowekyala
Copy link
Member

Also note that c[0] is not a valid resource. Javac errors with the following message:

the try-with-resources resource must either be a variable declaration or an expression denoting a reference to a final or effectively final variable.

The only expressions allowed here are field accesses and variable accesses according to the JLS 14. It seems javac also accepts this, but that's all. So while we should support this corner case, I don't think we should remove the parser check altogether.

@jsotuyod jsotuyod added the has:pr label Apr 5, 2024
@adangel adangel modified the milestones: 7.1.0, 7.2.0 Apr 25, 2024
@adangel adangel removed the has:pr label May 2, 2024
@adangel adangel closed this as completed in 03b8d1a May 2, 2024
adangel added a commit that referenced this issue May 2, 2024
Merge pull request #4934 from oowekyala:issue4912-java-grammar-fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug PMD crashes or fails to analyse a file. in:grammar About the grammar of a lexer or parser, eg, a parse/lex exception
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants