Conversation
oowekyala
left a comment
There was a problem hiding this comment.
Not a thorough review, just a couple of comments. Keep up the good work
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AntlrBaseNode.java
Outdated
Show resolved
Hide resolved
|
|
||
| private Statement withBefores(Statement statement) { | ||
| List<FrameworkMethod> befores = getTestClass().getAnnotatedMethods(Before.class); | ||
| private Statement withBefores(final Statement statement) { |
There was a problem hiding this comment.
Note to maintainers: We should really make it clear where we expect final modifiers on local vars & parameters in our codebase. I think it's just noise in short methods, especially when it pollutes a diff like that.
There was a problem hiding this comment.
There is a PMD rule that make finals required :P It's not enabled on this project though.
Generated by 🚫 Danger |
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AntlrBaseNode.java
Outdated
Show resolved
Hide resolved
...n/java/net/sourceforge/pmd/lang/swift/rule/bestpractices/ProhibitedInterfaceBuilderRule.java
Show resolved
Hide resolved
|
@jsotuyod Thanks for the review! we will add guidance for developers regarding the rules we added, and provide the examples. Regarding the |
- Remove Context suffix from Antlr grammar XPath names
|
@tomidelucca @matifraga I took the liberty of refactoring how you handled terminal nodes. I also turned the rule to a rulechain rule and found rule chain indexation was not working, so had to fix that. Please review yourselves before I merge. @oowekyala I added a comment on |
|
Nice catch! @jsotuyod I'm ok with the changes, we weren't aware of the rule chain related code on |
Before submitting a PR, please check that:
master. The PMD team will merge back to support branches as needed../mvnw clean verifypasses. This will build and test PMD, execute PMD and checkstyle rules. Check this for more infoPR Description:
This PR adds an XPath and a visitor rule for the Swift language.