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

Improved CustomDeclarationOrderCheck #295

Merged
merged 1 commit into from
Jan 6, 2015
Merged

Improved CustomDeclarationOrderCheck #295

merged 1 commit into from
Jan 6, 2015

Conversation

damianszczepanik
Copy link
Member

  • Added tests to increase coverage
  • Fixed rule in test method
  • Fixed formatting (indents)

Pull #295

- Added tests to increase coverage
- Fixed rule in test method
- Fixed formatting (indents)

Pull #295
@coveralls
Copy link

Coverage Status

Coverage increased (+1.18%) when pulling 15cd0e8 on damianszczepanik:CustomDeclarationOrderCheck into d2c4c45 on sevntu-checkstyle:master.

@damianszczepanik
Copy link
Member Author

OK, let me summarize reason of changes:

  1. I found that rule "MainMethod()" is not parsed and it should be "MainMethod(.*)" as it is mentioned in default order https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java#L200
    Since 'main' method has only one valid signature (or rather two: with [] and ...) so using rule "MainMethod()" should ends with error. Currently both are accepted but only one is parsed.
  2. Added "this" to field because one branch was not covered (expect to have DOT token)
  3. Added new test to cover branch that triggers wrong "MainMethod()" order.

Finally it improves coverage by +1.18% :-)

@romani romani merged commit 15cd0e8 into sevntu-checkstyle:master Jan 6, 2015
@romani
Copy link
Member

romani commented Jan 6, 2015

thanks a lot !

@damianszczepanik damianszczepanik deleted the CustomDeclarationOrderCheck branch January 6, 2015 21:00
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.

3 participants