Skip to content

[java] Update CommentDefaultAccessModifier#3188

Merged
adangel merged 2 commits into
pmd:pmd/7.0.xfrom
oowekyala:update-CommentDefaultAccessModifier
Apr 10, 2021
Merged

[java] Update CommentDefaultAccessModifier#3188
adangel merged 2 commits into
pmd:pmd/7.0.xfrom
oowekyala:update-CommentDefaultAccessModifier

Conversation

@oowekyala

Copy link
Copy Markdown
Member

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala added this to the 7.0.0 milestone Apr 2, 2021
@oowekyala oowekyala changed the base branch from master to pmd/7.0.x April 2, 2021 13:11
@ghost

ghost commented Apr 2, 2021

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 6298 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 9096 violations,
introduces 8183 new violations, 1 new errors and 0 new configuration errors,
removes 15644 violations, 10 errors and 2 configuration errors.
Full report

Generated by 🚫 Danger


/**
* Returns true if this is a regular interface declaration (not an annotation).
* Note that {@link #isInterface()} counts annotations in.

@adangel adangel Apr 10, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure, if we should do it the other way round. If you ask be in the middle of the night, whether Interface should return true as "isInterface" and what Annotation would return. I'd tell you, Annotations should return false....

I want to say, on first glance, I'd say, it's easier to have "isInterface" and "isAnnotation" rather than "isInterface", "isAnnotation" and "isRegularInterface"...

But I see, we also have "isRegularClass"....

We still have the possibility to change the semantics of those APIs in PMD 7

Update: Or in other words: What is less surprising? That isInterface returns true for annotations or that it returns false?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it's true that isInterface may be surprising, especially since in pmd 6 we have ASTClassOrInterfaceDeclaration#isInterface which is basically this isRegularInterface method. But it's in line with the JClassSymbol#isInterface, JTypeMirror#isInterface (and ultimately java.lang.Class#isInterface, after which the other two are modeled)... And this way !isInterface() actually means isClass(), with the same meaning as JClassSymbol#isClass vs JClassSymbol#isInterface. If we exclude annotations then !isInterface() means "is class or annotation", which doesn't seem very useful

@adangel adangel self-assigned this Apr 10, 2021
@adangel adangel merged commit 1ddc5c6 into pmd:pmd/7.0.x Apr 10, 2021
@oowekyala oowekyala deleted the update-CommentDefaultAccessModifier branch April 10, 2021 17:47
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
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.

2 participants