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] Upgrade to ASM7 for JDK 11 support #1521

Merged
merged 1 commit into from Dec 12, 2018

Conversation

markpritchard
Copy link
Contributor

Before submitting a PR, please check that:

  • The PR is submitted against master. The PMD team will merge back to support branches as needed.
  • ./mvnw clean verify passes. This will build and test PMD, execute PMD and checkstyle rules. Check this for more info

PR Description:

Remove ASM7_EXPERIMENTAL flag since ASM7 has been released.

PMD 6.10.0 passes the ASM7_EXPERIMENTAL flag to asm, which fails on asm 6.2.1 - ClassVisitor enforces only stable ASM per https://gitlab.ow2.org/asm/asm/blob/master/asm/src/main/java/org/objectweb/asm/ClassVisitor.java#L69

Upgrading PMD to use the recently released ASM 7.0 library and replacing ASM7_EXPERIMENTAL with ASM7 works.

Remove ASM7_EXPERIMENTAL flag since ASM7 has been released.
@pmd-test
Copy link

1 Message
📖 This changeset introduces 0 new violations and 0 new errors,
removes 0 violations and 0 errors. Full report

Generated by 🚫 Danger

@adangel adangel self-assigned this Dec 12, 2018
@adangel adangel added this to the 6.11.0 milestone Dec 12, 2018
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good!

Just a remark/question:

PMD 6.10.0 passes the ASM7_EXPERIMENTAL flag to asm, which fails on asm 6.2.1 - ClassVisitor
enforces only stable ASM per https://gitlab.ow2.org/asm/asm/blob/master/asm/src/main/java/org/objectweb/asm/ClassVisitor.java#L69

Since PMD 6.10.0 uses asm 6.2.1, passing ASM7_EXPERIMENTAL should still work and JDK 11 support should not be broken.
Of course, when we upgrade asm, we need to change this API flag (like you did).

Did you experience any problems with JDK 11 and PMD 6.10.0 so far?

@adangel adangel merged commit e357da1 into pmd:master Dec 12, 2018
adangel added a commit that referenced this pull request Dec 12, 2018
@markpritchard
Copy link
Contributor Author

Oops, sorry @adangel - I meant to say fails on asm 7

I haven't seen any issues at all with PMD 6.10.0 on JDK 11 (or with JDK 11 in general).

Thanks again for all your great work!

Cheers,

Mark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants