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

Add Rule configuration: DisableSyntax.regex #494

Merged
merged 3 commits into from
Dec 12, 2017

Conversation

MasseGuillaume
Copy link
Contributor

No description provided.

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Nice! LGTM 👍

}
}
val tokensLintMessage =
ctx.tree.tokens.collect {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/.collect/.traverse/ and build directly to regexLintMessages? Either way is fine.

@@ -10,16 +10,40 @@ class IntervalSet(range: BitSet) {
val otherRange = BitSet((start to end): _*)
(range & otherRange).nonEmpty
}

override def toString: String = {
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general note, it's best to try and open separate PRs for unrelated changes. I know it requires more fiddling with branches locally but it makes it easier to review, esp. for larger PRs at least. It's fine here, just wanted to mention this.

def foo: Unit = return // assert: DisableSyntax.keywords.return
throw new Exception("ok") // assert: DisableSyntax.keywords.throw

"semicolon"; // assert: DisableSyntax.noSemicolons
<a>xml</a> // assert: DisableSyntax.noXml
// assert: DisableSyntax.noTabs

implicit class StringPimp(value: String) { // assert: DisableSyntax.[P|p]imp
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I wonder if CustomMessage should have an optional id field to avoid exposing complex messages like this into linter messages. Opened #495

@MasseGuillaume MasseGuillaume force-pushed the disable-regex branch 2 times, most recently from 5c844b5 to 3adaad7 Compare December 11, 2017 18:05
Before:

scala/test/DisableSyntax.scala:21:3: error: [DisableSyntax.keywords.null]:
keywords.null is disabled
  null
  ^

Now:

 scala/test/DisableSyntax.scala:21:3: error: [DisableSyntax.keywords.null]:
null is disabled
  null
  ^
@olafurpg olafurpg merged commit e4a5c35 into scalacenter:master Dec 12, 2017
@MasseGuillaume MasseGuillaume deleted the disable-regex branch December 12, 2017 15:34
MasseGuillaume added a commit to MasseGuillaume/scalafix that referenced this pull request Dec 20, 2017
```
> scalafix --diff-base <tab>
HEAD
ag/gh-pages
ag/issue-260
ag/issue-319
ag/master
...
v0.5.5
v0.5.6
|00| 51abefa -- wip improve sbt completion (13 hours ago)
|01| 3f63b5b -- Merge pull request scalacenter#474 from marcelocenerine/trim_includeNewLine (13 hours ago)
|02| 47b94c0 -- Add leadingSpaces/trailingSpaces methods to TokenList (13 hours ago)
|03| 8bd026d -- Merge pull request scalacenter#503 from MasseGuillaume/feature/368-escape-patches (13 hours ago)
|04| 20e445f -- Escape hatch on Patch (fix scalacenter#368) (13 hours ago)
|05| a2c5d70 -- Merge pull request scalacenter#500 from MasseGuillaume/feature/495-custom-id (13 hours ago)
|06| 59efe7d -- Add CustomMessage to the public api (13 hours ago)
|07| 9ae6071 -- Add id for CustomMessage (fix scalacenter#495) (13 hours ago)
|08| e4a5c35 -- Merge pull request scalacenter#494 from MasseGuillaume/disable-regex (13 hours ago)
|09| a422860 -- Merge pull request scalacenter#497 from olafurpg/disable-signatures (13 hours ago)
|10| 7930947 -- DisableSyntax add regex (13 hours ago)
|11| 5dbdd6b -- IntervalSet test for empty and add toString (13 hours ago)
|12| b022fbd -- DisableSyntax don't repeat DisableSyntax.keyword in message (13 hours ago)
|13| a992b02 -- Assert instead of scalafix:ok (13 hours ago)
|14| 7896ccd -- Refactor Disable to use views. (13 hours ago)
|15| 58acdbe -- Fix scalacenter#493, handle synthetics and symbol signatures in Disable. (13 hours ago)
|16| b48d7f0 -- Merge pull request scalacenter#490 from olafurpg/unmanagedSources (13 hours ago)
|17| e9b2b0a -- s/canFormat/canFix/ (13 hours ago)
|18| 26be6fa -- Use unmanagedSources instead of unmanagedSourceDirectories. (13 hours ago)
|19| 4d46001 -- Merge pull request scalacenter#488 from olafurpg/master (13 hours ago)
```
MasseGuillaume added a commit to MasseGuillaume/scalafix that referenced this pull request Dec 22, 2017
```
> scalafix --diff-base <tab>
HEAD
ag/gh-pages
ag/issue-260
ag/issue-319
ag/master
...
v0.5.5
v0.5.6
|00| 51abefa -- wip improve sbt completion (13 hours ago)
|01| 3f63b5b -- Merge pull request scalacenter#474 from marcelocenerine/trim_includeNewLine (13 hours ago)
|02| 47b94c0 -- Add leadingSpaces/trailingSpaces methods to TokenList (13 hours ago)
|03| 8bd026d -- Merge pull request scalacenter#503 from MasseGuillaume/feature/368-escape-patches (13 hours ago)
|04| 20e445f -- Escape hatch on Patch (fix scalacenter#368) (13 hours ago)
|05| a2c5d70 -- Merge pull request scalacenter#500 from MasseGuillaume/feature/495-custom-id (13 hours ago)
|06| 59efe7d -- Add CustomMessage to the public api (13 hours ago)
|07| 9ae6071 -- Add id for CustomMessage (fix scalacenter#495) (13 hours ago)
|08| e4a5c35 -- Merge pull request scalacenter#494 from MasseGuillaume/disable-regex (13 hours ago)
|09| a422860 -- Merge pull request scalacenter#497 from olafurpg/disable-signatures (13 hours ago)
|10| 7930947 -- DisableSyntax add regex (13 hours ago)
|11| 5dbdd6b -- IntervalSet test for empty and add toString (13 hours ago)
|12| b022fbd -- DisableSyntax don't repeat DisableSyntax.keyword in message (13 hours ago)
|13| a992b02 -- Assert instead of scalafix:ok (13 hours ago)
|14| 7896ccd -- Refactor Disable to use views. (13 hours ago)
|15| 58acdbe -- Fix scalacenter#493, handle synthetics and symbol signatures in Disable. (13 hours ago)
|16| b48d7f0 -- Merge pull request scalacenter#490 from olafurpg/unmanagedSources (13 hours ago)
|17| e9b2b0a -- s/canFormat/canFix/ (13 hours ago)
|18| 26be6fa -- Use unmanagedSources instead of unmanagedSourceDirectories. (13 hours ago)
|19| 4d46001 -- Merge pull request scalacenter#488 from olafurpg/master (13 hours ago)
```
@MasseGuillaume MasseGuillaume changed the title Add regex to the DisableSyntax rule Add Rule configuration: DisableSyntax.regex Jan 25, 2018
@MasseGuillaume MasseGuillaume added this to the v0.5.8 milestone Jan 25, 2018
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