maxvetrenko / checkstyle

Checkstyle is a development tool to help programmers write Java code that adheres to a coding standard. By default it supports the Sun Code Conventions, but is highly configurable. It can be invoked with an ANT task and a command line program.
http://checkstyle.sourceforge.net/
GNU Lesser General Public License v2.1
0 stars 0 forks source link

New Check: NoLineWrapCheck #20

Closed maxvetrenko closed 10 years ago

maxvetrenko commented 10 years ago

http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.3.2-import-line-wrapping http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.2-package-statement

The package and import statements are not line-wrapped.

romani commented 10 years ago

please make sure that list of tokenType that could not be split in few lines could be specified and adjusted by user.

romani commented 10 years ago

1) HTML site should be updated to have documentation.

2) remove "@version" from javadoc

3) keep only one example of usage, default example could be removed.

4) please do squash of all commit in one - I need to see final changes.

maxvetrenko commented 10 years ago

1) HTML site should be updated to have documentation. How to update HTML?

maxvetrenko commented 10 years ago
  1. Done
  2. Done
  3. Done
maxvetrenko commented 10 years ago

Updated site, code coverage is 100%

romani commented 10 years ago

1)

avoid to wrap import ....

Should be "restrict to wrap ....."

2) please provide Examples of line-wrapped import and package and how Check force it looks like . Remember that users understand better by example rather then by academic description.

3) I do not see changes for xdoc files (we site).

maxvetrenko commented 10 years ago

Updated xdoc files

rdiachenko commented 10 years ago

... but it's possible to check any statement. return new int[] {TokenTypes.PACKAGE_DEF, TokenTypes.IMPORT};

Where is the truth?

Examples of line-wrapped import and package:

Change to "Examples of line-wrapped statements"

Examples without line-wrapping import and package statements:

Change to "Examples of not line-wrapped statements"

  • @Test
    • public void testBadCaseWithTokenConfig() throws Exception
    • {
    • final DefaultConfiguration checkConfig = createCheckConfig(NoLineWrapCheck.class);
    • checkConfig.addAttribute("tokens", "IMPORT");
    • final String[] expected = {
    • "6: import statement should not be line-wrapped.",
    • };
    • verify(checkConfig, getPath("whitespace/NoLineWrapBadInput.java"), expected);
    • }

I don't understand what are you trying to test here. Your previous test already covers such a case.

rdiachenko commented 10 years ago

final DefaultConfiguration checkConfig = createCheckConfig(NoLineWrapCheck.class);

  • checkConfig.addAttribute("tokens", "IMPORT");

Could you add other tokens (class, method, fields, etc.) and extend your testinput in order so we can see that other statemens are also covered.

maxvetrenko commented 10 years ago

Done.

rdiachenko commented 10 years ago

@Override

  • public int[] getRequiredTokens() {
    1. Use public int[] getAcceptableTokens() method instead
    2. Update a list of acceptable tokens. Look at all TokenTypes.*_DEF. Does it make sense to include them all?

public void testGoodCase() public void testBadCase() public void testBadCaseWithTokenConfig()

Bad names. Each test has to have a clear name which reflects its purpose.

maxvetrenko commented 10 years ago

Use public int[] getAcceptableTokens() method instead

I don't agree with you. getRequiredTokens return tokens that MAY set as a property and getAcceptableTokens uses to protect getDefaultTokens.

Does it make sense to include them all?

I think, that it make sense to include method, class, enum, ctor and interface

Bad names. Each test has to have a clear name which reflects its purpose.

Done. Check it

rdiachenko commented 10 years ago

By default this Check

  • * restrict to wrap import and package statements

Change to "By default this Check restricts wrapping import and package statements"

testCaseWithLineWrapping()

Change to "testDefaultTokensLineWrapping"

testCaseWithLineWrappingAndCustomTokensToCheck()

Change to "testCustomTokensLineWrapping"

rdiachenko commented 10 years ago

By default this Check restrict to wrap import and package statements

Change to "By default this Check restricts wrapping import and package statements"

Please change it everywhere

romani commented 10 years ago

1)

Examples of not line-wrapped statements:

JavaDoc is not a place for code examples! All example a re placed for reason. please provide config that force code to be like this.

maxvetrenko commented 10 years ago
  1. Done
romani commented 10 years ago
  1. Done

I do not see any meaningful changes, that will bind example configuration and examples

maxvetrenko commented 10 years ago

It's my changes: commit. If something is wrong, let's discuss via Skype.

romani commented 10 years ago

ok for PR, please donot forget to create issue in Checkstyle repo and make a link to it in commit message.

maxvetrenko commented 10 years ago

Merged