Add configuration for trailing_whitespace to ignore comments

[jaherhi]

Fixes #576

Rule trailing_whitespace can now be configured to ignore comments using ignores_comments, its default value is true

[codecov-io]

Current coverage is 85.67% (diff: 95.74%)

Merging #854 into master will increase coverage by 0.17%

@@             master       #854   diff @@
==========================================
  Files           108        108          
  Lines          4690       4767    +77   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           4010       4084    +74   
- Misses          680        683     +3   
  Partials          0          0          

Powered by Codecov. Last update e8771d9...c1df008

[marcelofabri]

I think you should use SyntaxKinds instead of checking for //. Also, I think this implementation won't work in "real" examples because it only checks the beginning of strings, i.e. this would fail:

class Example {
  // empty but with trailing spaces          
}

You probably can use something like this to check if the line only contains comments instead.

[jaherhi]

Thanks for the suggestion @marcelofabri! I managed to change my implementation to use SyntaxKind, although I'm afraid I don't get what you mean with the example, and I'm trying to see if that could happen using SyntaxKind too.

In your example, checking for // at the beginning of the string would detect that second line as a comment and, by default, ignore it even if it has trailing spaces, wouldn't it?

[norio-nomura]

Thank you for doing this! 🙏
It looks there is some performance regression that may be resolvable.
Could you please check my feedbacks?

I also found false negative pattern with block comment:

  let nsError = NSError(domain: NSCocoaErrorDomain,
                   code: NSFileNoSuchFileError,
                   userInfo: [
                     AnyHashable(NSFilePathErrorKey) : "/dev/null",
/* this line */      AnyHashable(NSStringEncodingErrorKey): /*ASCII=*/1,    
                   ])

[norio-nomura]

commentKinds is not need to be Set.

[norio-nomura]

lineContainsComment should be calculated only if $0.content.hasTrailingWhitespace() and configuration.ignoresComments are true.
Please pay attention to reduce calculation. 🕵🏻

[jaherhi]

Changes made to address the feedback 🙂 Thank you!

[norio-nomura]

I found false negative pattern that block comment has trailing whitespace:

let string1 = "string1"  /* block comment has trailing whitespace */  

But, maybe that won't be problems. 😃

So, it looks mostly good to me other than my comments.
Could you please check them?

[norio-nomura]

On non violating case, $0.content.hasTrailingWhitespace() is evaluated twice. The closure should return false first if $0.content.hasTrailingWhitespace() returns false.

[norio-nomura]

Same with my comment on validateFile(_:).
This loop does not need to evaluate following operations if line.content.hasTrailingWhitespace() returns false.

if !line.content.hasTrailingWhitespace() {
    correctedLines.append(line.content)
    continue
}

[jaherhi]

Changes made. Thanks for your comments and patience, @norio-nomura!

I can't think of a way to fix that without manually checking if the comment contains */and whitespace after it. Any suggestion, if that's a case that should work properly?

[norio-nomura]

We can continue with filing issue of the false negative and another PR. 😉
I want to merge this PR. But I forgot about CHANGELOG.md.
Could you please add a changelog entry, following the guidelines in CONTRIBUTING.md?

[jaherhi]

CHANGELOG.mdupdated. I rebased the branch to avoid conflicts after updating it.

[norio-nomura]

Thanks! 🙏

[jpsim]

Awesome! 🎉