Respect @SuppressWarnings to disable rules #620

marcelocenerine · 2018-02-21T01:19:08Z

This PR adds support to the @SuppressWarnings annotation as a hierarchical mechanism to disable Scalafix rules (#442).

This new mechanism is supported in all locations where Scala allows the annotation to be placed:

class
trait
object
type
methods
constructors (primary and secondary)
method/constructor parameters
val and vars

One or more rules can be specified in each annotation (more than one annotation can be applied to a given definition):

@SuppressWarnings(Array("rule1"))
@SuppressWarnings(Array("rule2"))
def foo(): Unit = ???

or

@SuppressWarnings(Array("rule1", "rule2"))
def foo(): Unit = ???

Rule names can be optionally prefixed with scalafix:. If they are, then Scalafix is able to report warnings for unused/redundant rules.

The standard wildcard value all can be used to disable all rules and is equivalent to // scalafix: off.

@SuppressWarnings can be combined with comment anchors on the same source file. If there is any overlapping, the annotation will take precedence. In that case, the overlapping anchor is only ignored within the scope of the annotated definition. (see discussion on #442 about this. Also, see example scenarios in the AnchorAnnotationMixed test file).

NOTE: The documentation has not been updated yet. I will wait until reviewers are happy with the code changes then I'll do it.

Implementation details:

`SuppressWarnings` vs. `@java.lang.SuppressWarnings`

As @olafurpg mentioned on #442, annotations require compilation in order to be 100% confident about their types (@java.lang.SuppressWarnings in this case). Given that Scalafix supports both syntactic and semantic rules and the type information is only available for the latter, we need to assume that SuppressWarnings (when in a Mod.Annot) resolves to @java.lang.SuppressWarnings.

Refactoring

In order to accomodate the implementation of this feature, I had to move some things around and do some cleanup. This may make EscapeHatch a little hard to review and it may be a good idea take a look at each commit individually, or else I can try to split it into multiple PRs. The main changes were:

moved all anchor-related code into a new inner class AnchoredEscapes. The anchor parsing logic was consolidated in the AnchoredEscapes companion object.
eliminated one step in the anchor parsing logic. Currently it transforms tree/tokens -> Escape -> EscapeFilter. The intermediary step didn't seem to add much benefit, so I merged it with the last one. This helped to eliminate a plenty of lines of code without compromising readability (my opinion). I may reconsider this if you don't agree.
all code related to SuppressWarnings handling was implemented in a new inner class AnnotatedEscapes and its corresponding companion object.
each EscapeFilter now corresponds to only a single rule rather than all the rules in one comment/annotation. The reason for that is explained in the improvement below.
Now EscapeHatch only cares about whether a rule is enabled or not. In order to decide that, it delegates to the 2 classes above according to the order of precedence.

Improvement: Warnings to point to the exact position of unused rules

Scalafix shows warnings for enable/disable rules that are unused. However, the warning messages don't mention the rule name nor the caret ("^") points to location of the unused rule. Instead, they point to the beginning of the anchor comment. When there are multiple unused rules in one anchor, the exact same warning is printed multiple times which does not help much. Please see example here: https://gist.github.com/marcelocenerine/619a52ef03f3872fbcd7e29dc8d921f3

I thought it would be more helpful to know the exact rule that is causing the warning so that users can go and remove it rather than try to manually figure out which one is causing the warning. This is probably even more relevant for the @SuppressWarnings case. See example: https://gist.github.com/marcelocenerine/8ab0358781826078d10a0acee786b4b7

Not sure if you agree but I made this improvement (I can take it out if you don't like). Now the caret points to the exact position of the unused rules when there is any, otherwise to the beginning of the comment. I also changed the warning messages to be more meaningful. See example: https://gist.github.com/marcelocenerine/b22bb526d012b783a8ad40aa41d54bba

Besides making the warning messages more helpful, this improvement also helped to uncovered 2 potential bugs.

Bugs

comma (,) not a separator for rule names in comments (https://scalacenter.github.io/scalafix/docs/users/configuration#selectively-disabling-rules):

{
  val x = null
  val aDummy = 0
  a.get
} // scalafix:ok EscapeHatchNoNulls, EscapeHatchDummyLinter, Disable.get

Scalafix does not ignore the first 2 rules:

scala/test/escapeHatch/Baz.scala:25:13: error: [EscapeHatchNoNulls]:
Nulls are not allowed.
    val x = null
            ^

---------------------------------------

scala/test/escapeHatch/Baz.scala:26:9: error: [EscapeHatchDummyLinter]:
Dummy!
    val aDummy = 0
        ^

NoInfer.any2stringadd:
The code below does not trigger the rule:

1 + "foo"

whereas this does:

Some(1) + "foo"

There was a test relying on this, which should have failed but was passing because of the 1st bug. I did not investigate this one further, so if you think there is really something wrong then let's file a bug.

marcelocenerine · 2018-02-21T01:32:11Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorExpression.scala

@@ -37,7 +37,7 @@ object EscapeHatchExpression {

  (
    null,
-    1 + "foo",
+    Some(1) + "foo",
    a.get
  ) // scalafix:ok NoInfer.any2stringadd, Disable.get



this is the test where NoInfer.any2stringadd does not get triggered but still passes due to the bug in the regex

Aaah, lol. Nice catch!

Should there have been an "unused warning" on NoInfer.any2stringadd?

Yes indeed. But the bug in the regex was preventing it from happening

marcelocenerine · 2018-02-21T01:32:44Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorMultipleRules.scala


  // scalafix:off NoInfer.any2stringadd, Disable.get
-  1 + "foo"
+  Some(1) + "foo"

  val aDummy = 0 // assert: EscapeHatchDummyLinter


olafurpg · 2018-02-21T07:52:05Z

Thank you for this contribution @marcelocenerine nice work! 😄

I'll let @MasseGuillaume take a first pass since he developed EscapeHatch where the biggest diff seems to be

MasseGuillaume · 2018-02-21T11:22:23Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorAnnotationMixed.scala

+  def aDummy4(x: Option[Any]): Unit = {
+    /* scalafix:off EscapeHatchDummyLinter */ // assert: UnusedScalafixSuppression.Disable
+    val bDummy = 0
+    // scalafix:on EscapeHatchDummyLinter


This one is counter-intuitive.

I suggest the following:

@SuppressWarnings(Array("EscapeHatchDummyLinter")) def aDummy4(x: Option[Any]): Unit = { // scalafix:on EscapeHatchDummyLinter val bDummy = 0 // assert EscapeHatchDummyLinter // scalafix:off EscapeHatchDummyLinter val cDummy = 0 }

Optionnaly, we might also add this:

@SuppressWarnings(Array("EscapeHatchDummyLinter")) def aDummy4(x: Option[Any]): Unit = { val bDummy = 0 // scalafix:notOk EscapeHatchDummyLinter //assert EscapeHatchDummyLinter }

this is because the annotation takes precedence when there is overlapping (only within the region where the overlap takes place. Comments are still respected if they start before/after). See my comment #442 (comment)

(there is a counterexample in the comment I mentioned above)

Yup, I got that. I think we can handle the case where the anchor range position is completely inside the annotation range position.

Also:

// scalafix:off EscapeHatchDummyLinter def aDummy = {} // scalafix:on EscapeHatchDummyLinter @SuppressWarnings(Array("EscapeHatchDummyLinter")) def bDummy = {} @SuppressWarnings(Array("EscapeHatchDummyLinter")) def cDummy = {}

Could have the same effect as:

// scalafix:off EscapeHatchDummyLinter def aDummy = {} // scalafix:on EscapeHatchDummyLinter // scalafix:off EscapeHatchDummyLinter def bDummy = {} // scalafix:on EscapeHatchDummyLinter // scalafix:off EscapeHatchDummyLinter def cDummy = {} // scalafix:on EscapeHatchDummyLinter

It would use the same logic we have in // scalafix: ok

ok, let me put some thoughts on this. @olafurpg, would you like to chip in?

I agree with @marcelocenerine 's assessment in #442 (comment) that it is counter-intuitive if annotations are treated as comments. Annotations should have push/pop semantics and they should override comments.

so should I keep the current behavior then? I updated the docs last night.

Yes, let's keep the current behavior and document the potentially counter-intuitive behavior that annotations take precedence over comments. I'm wondering, can we report an unused warning on the scalafix:ok comment with potentially an additional message that says something like "Note. This comment is unused because of the enclosing @SuppressWarning and annotations take precedence over comments."

That is definitely doable. The only thing is that it would require the comment type (on+off/ok) to be preserved. Currently that information gets lost very early as // scalafix:ok is transformed into a pair of off/on EscapeFilters. If you think this is a nice to have, then I can invest some time on it.

MasseGuillaume · 2018-02-21T12:18:03Z

Really good PR, I'm really impressed by the quality of this work.

I found another bug on scalafix:ok while reviewing this. I'm creating a separate issue to fix it (#627).

object A {
  val a = null
} // scalafix:ok DisableSyntax.keywords.null

[fast2] Running DisableSyntax
/home/gui/fast2/src/main/scala/a.scala:4:11: error: [DisableSyntax.keywords.null] null is disabled
  val a = null
          ^
/home/gui/fast2/src/main/scala/a.scala:5:3: warn: [UnusedScalafixSupression.Disable] This comment does not disable any rule
} // scalafix:ok DisableSyntax.keywords.null
  ^

olafurpg · 2018-02-26T09:49:58Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorAnnotationMixed.scala

@@ -0,0 +1,87 @@
+/*
+rules = [
+  "class:scalafix.test.EscapeHatchDummyLinter"


What do you think about renaming "EscapeHatchDummyLinter" to "NoDummy" and "EscapeHatchNoNulls" to "NoNull"?

Alternatively, s/EscapeHatchDummyLinter/EscapeHatchNoDummy/ would also be an improvement

done (the shorter option)

olafurpg · 2018-02-26T09:54:33Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorAnnotationMixed.scala

+  def aDummy4(x: Option[Any]): Unit = {
+    /* scalafix:off EscapeHatchDummyLinter */ // assert: UnusedScalafixSuppression.Disable
+    val bDummy = 0
+    // scalafix:on EscapeHatchDummyLinter


Yes, let's keep the current behavior and document the potentially counter-intuitive behavior that annotations take precedence over comments. I'm wondering, can we report an unused warning on the scalafix:ok comment with potentially an additional message that says something like "Note. This comment is unused because of the enclosing @SuppressWarning and annotations take precedence over comments."

olafurpg · 2018-02-26T09:55:15Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorAnnotationMixed.scala

+  @SuppressWarnings(Array("EscapeHatchDummyLinter"))
+  def aDummy6(x: Option[Any]): Unit = {
+    val bDummy = 0
+    // scalafix:on EscapeHatchDummyLinter


Should this comment get an unused warning?

Aah, I guess it's used in line 73 right, makes sense. Let's hope in practice people won't mix comments too much with annotations 😅

olafurpg · 2018-02-26T09:58:00Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnchorExpression.scala

@@ -37,7 +37,7 @@ object EscapeHatchExpression {

  (
    null,
-    1 + "foo",
+    Some(1) + "foo",
    a.get
  ) // scalafix:ok NoInfer.any2stringadd, Disable.get



Should there have been an "unused warning" on NoInfer.any2stringadd?

olafurpg · 2018-02-26T10:00:01Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnnotationMultipleRules.scala

+*/
+package test.escapeHatch
+
+object AnnotationMultipleRules {


Very nice 👍

olafurpg · 2018-02-26T10:08:13Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnnotationScopes.scala

+  }
+
+  // Local variable
+  class DummyClass_9( // assert: EscapeHatchDummyLinter


Ditto. You can assume the position of the annotation enclosing jDummy below is correct, otherwise it would be a scalameta bug.

olafurpg · 2018-02-26T10:08:41Z

scalafix-tests/input/src/main/scala/test/escapeHatch/AnnotationUnused.scala

+  @SuppressWarnings(Array("scalafix:EscapeHatchNoNulls")) // assert: UnusedScalafixSuppression.Disable
+  val c = 0
+
+  @SuppressWarnings(Array("EscapeHatchDummyLinter", "EscapeHatchNoNulls")) // OK, not prefixed


olafurpg · 2018-02-26T10:13:47Z

website/src/main/tut/docs/users/configuration.md

+
+**Note:** Suppression via comments and `@SuppressWarnings` can be combined in the same source file. Be mindful not to 
+introduce overlaps between the two as it can cause confusion. Scalafix gracefully handles overlaps by given 


s/gracefully//

Should we add an example here? Might also want to add "it can cause confusion and counter-intuitive behavior".

done. Please see if the example is good enough or if you want a nastier combination

olafurpg · 2018-02-26T10:14:02Z

website/src/main/tut/docs/users/configuration.md


-It's possible to supress false positives linter message over source file regions using comments. This provides fine grained control over what to report
+## Suppressing rules


Great writing! You have a knack for docs

I just copied most of this stuff from your comment in #442 🤣

olafurpg · 2018-02-26T10:29:45Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

-          )
+      def hasSuppressWarnings(mods: List[Mod]): Boolean =
+        mods.exists {
+          case Mod.Annot(Init(Type.Name(SuppressWarnings), _, _)) => true


What happens for @java.lang.SuppressWarnings? I agree it makes sense to do this syntactically in order to enable syntactic rules like DisableSyntax to support annotations. Can we add an entry in the docs explaining the detection of @SuppressWarnings?

The @SuppressWarnings annotation is detected without compilation. Any annotation matching the syntax @SuppressWarnings(..) regardless if it's java.lang.SupressWarnings or not will trigger the suppression mechanism. This is done in order to support @SuppressWarnings for syntactic rules like DisableSyntax.

To support qualifying the annotation, I recommend we do the following:

case Mod.Annot(Init(Type.Name(SuppressWarnings), _, List(List(_)))) => true case Mod.Annot(Init(Type.Select(_, Type.Name(SuppressWarnings)), _, List(List(_)))) => true // `@java.lang.SuppressWarnings(..)`

What happens for @java.lang.SuppressWarnings?

Nice catch! Fixed

Can we add an entry in the docs explaining the detection of @SuppressWarnings?

Done

…offending rule

…Null

@SuppressWarnings

- Remove redundant expressions in AnnotationScopes test - Update docs to include example of mixing @SuppressWarnings and comments

olafurpg

Had another round of review. A few more comments, otherwise getting very close to merge! 😄

olafurpg · 2018-02-28T08:34:05Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+    val unusedDisableWarning = LintCategory
+      .warning(
+        "Disable",
+        "Rule(s) don't need to be disabled at this position. This can be removed!")


This comment has no effect and can be removed

?

olafurpg · 2018-02-28T08:34:36Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+
+    val unusedDisableWarning = LintCategory
+      .warning(
+        "Disable",


Let's make this "" so the category is only "UnusedScalafixSuppression"

I like it. Done

olafurpg · 2018-02-28T08:34:46Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+      .withOwner(UnusedScalafixSuppression)
+    val unusedEnableWarning = LintCategory
+      .warning(
+        "Enable",


olafurpg · 2018-02-28T08:35:08Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+    val unusedEnableWarning = LintCategory
+      .warning(
+        "Enable",
+        "Rule(s) not disabled at this position. This can be removed!")


This comment has no effect and can be removed

?

olafurpg · 2018-03-06T07:50:47Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

-            rule => unusedEnable += unusedEnableWarning.at(anchorPosition.value)
-          )
+      tree.foreach {
+        case t @ Defn.Class(mods, _, _, _, _) if hasSuppressWarnings(mods) =>


I've been meaning for a long time to add mods: List[Mod] on Member to avoid boilerplate like this, even more motivated now!

Actually, could we add a Mods extractor in TreeExtractors?

scalafix/scalafix-core/shared/src/main/scala/scalafix/util/TreeOps.scala

Line 11 in 67a1f28

object Select {

object Mods { def unapply(tree: Tree): Option[List[Mod]] = ... }

Seems like Decl.{Val,Var,Def,Type} are unhandled, it should be possible to annotate those as well.

Actually, could we add a Mods extractor in TreeExtractors?

Nice! Done

Seems like Decl.{Val,Var,Def,Type} are unhandled, it should be possible to annotate those as well.

For some reason I missed these ones. Thanks for catching this

olafurpg · 2018-03-06T07:52:18Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

-        case _ => ()
+    private def extractRules(mods: List[Mod]): List[(String, Position)] = {
+      def process(rules: List[Term]) = rules.map {
+        case lit @ Lit.String(rule) =>


What happens if this is not a string literal?

val x = "foo" @SuppressWarnings(Array(x))

I think it's fair to require string literals, but we should at least not crash if there is a variable.

oops. That's one corner case that I did not consider. I think there is no way to lookup the value. Am I right? I'm just avoiding the crash for now unless you have a better idea.

olafurpg · 2018-03-06T07:57:27Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+        case t @ Defn.Var(mods, _, _, _) if hasSuppressWarnings(mods) =>
+          addAnnotatedEscape(t, mods)
+
+        case t @ Term.Param(mods, _, _, _) if hasSuppressWarnings(mods) =>


Missing Type.Param

olafurpg · 2018-03-06T07:58:05Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+        case t @ Defn.Type(mods, _, _, _) if hasSuppressWarnings(mods) =>
+          addAnnotatedEscape(t, mods)
+
+        case t @ Defn.Def(mods, _, _, _, _, _) if hasSuppressWarnings(mods) =>


Missing Defn.Macro

olafurpg · 2018-03-06T07:58:27Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+        case t @ Ctor.Primary(mods, _, _) if hasSuppressWarnings(mods) =>
+          addAnnotatedEscape(t, mods)
+
+        case t @ Ctor.Secondary(mods, _, _, _, _)


Missing Pkg.Object

done. However, I did not add tests as annotations are not permitted on package objects afaik

marcelocenerine · 2018-03-07T01:19:15Z

@olafurpg thanks for taking a look at this PR again and for providing great suggestions :)

I addressed all comments and updated the PR, so it's ready for another round. Please let me know when you are happy with it so that I can squash all the commits before merging it (unless gh let you do it yourself). Thanks

marcelocenerine · 2018-03-07T08:56:50Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

+  private val UnusedScalafixSuppression = RuleName("UnusedScalafixSuppression")
+  private val UnusedWarning = LintCategory
+    .warning("", "This comment has no effect and can be removed")
+    .withOwner(UnusedScalafixSuppression)


Actually, the term "comment" is not 100% accurate here given that this warning message can also refer to a rule name in the annotation. Maybe we can say "escape" or "suppression" instead?

Please see if the updated warning message is generic enough for all positions/scenarios where it can refer to:

specific rule in an annotation:

scala/test/escapeHatch/AnnotationPrefixedRules.scala:23:47: error: [UnusedScalafixSuppression]: Unused Scalafix suppression. This can be removed @SuppressWarnings(Array("scalafix:NoNull", "scalafix:NoDummy")) ^

wildcard ON comment:

scala/test/escapeHatch/AnchorMultipleRules.scala:31:3: error: [UnusedScalafixSuppression]: Unused Scalafix suppression. This can be removed // scalafix:on ^

wildcard OFF comment:

scala/test/escapeHatch/AnchorWildcard.scala:16:3: error: [UnusedScalafixSuppression]: Unused Scalafix suppression. This can be removed // scalafix:off ^

specific rule in an ON comment:

scala/test/escapeHatch/AnchorMultipleRules.scala:30:18: error: [UnusedScalafixSuppression]: Unused Scalafix suppression. This can be removed /* scalafix:on NoInfer.any2stringadd, Disable.get */ ^

specific rule in an OFF comment:

scala/test/escapeHatch/AnchorMultipleRules.scala:22:42: error: [UnusedScalafixSuppression]: Unused Scalafix suppression. This can be removed // scalafix:off NoInfer.any2stringadd, Disable.get ^

Good catch! "comment" is indeed misleading, I like the new phrasing better 👍

olafurpg

LGTM. Very impressive PR @marcelocenerine 👏 👏

You will be able to use this change via SNAPSHOT as soon as CI completes after merge (https://scalacenter.github.io/scalafix/docs/users/installation#snapshot). However, we are maybe 2 weeks away from having a more stable release on Maven. There are quite a few more pending changes related to recent semanticdb improvements.

Re squashing the history: don't bother. I prefer to keep the git history more detailed to understand the motivation of each change. We already don't run CI at every commit, which admittedly makes bisect hard but that's the price we pay.

olafurpg · 2018-03-12T16:05:47Z

scalafix-core/shared/src/main/scala/scalafix/util/TreeOps.scala

@@ -15,6 +15,29 @@ object TreeExtractors {
      case _ => None
    }
  }
+
+  object Mods {


olafurpg · 2018-03-12T16:10:15Z

scalafix-core/shared/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

@@ -251,11 +210,13 @@ object EscapeHatch {
      }

    private def extractRules(mods: List[Mod]): List[(String, Position)] = {
-      def process(rules: List[Term]) = rules.map {
+      def process(rules: List[Term]) = rules.flatMap {


FYI .collect is a good fit here

that's true... too late now. I can update it as part of a future PR

marcelocenerine · 2018-03-12T18:19:53Z

Awesome! Thanks @olafurpg and @MasseGuillaume for your great input on this PR 👏

marcelocenerine force-pushed the suppress_warnings branch from 41659d0 to bfbefe2 Compare February 21, 2018 01:30

marcelocenerine commented Feb 21, 2018

View reviewed changes

olafurpg requested a review from MasseGuillaume February 21, 2018 07:49

MasseGuillaume suggested changes Feb 21, 2018

View reviewed changes

marcelocenerine force-pushed the suppress_warnings branch from 7fe609a to 45596a8 Compare February 22, 2018 01:26

olafurpg suggested changes Feb 26, 2018

View reviewed changes

marcelocenerine added 13 commits February 27, 2018 22:41

fix typos: toogle -> toggle; supress -> suppress

c861d85

rename tests starting with EscapeHatch* to Anchor*

5d9ab59

Refactor existing logic to accommodate basic impl. of @SuppressWarnings

e0475ff

add support to @SuppressWarnings with 'all'

a0cf99c

add support to rule names prefixed with 'scalafix:' in @SuppressWarnings

434a0ec

Makes warnings more precise by pointing to the exact position of the …

f0a0976

…offending rule

Update scaladocs

2aa0b8b

Enable warnings for unused rules in @SuppressWarnings

c66f8e5

Make internal members private

06f2513

Update splitRules regex

f51b94a

Update docs regarding rules suppression

27c3468

Rename EscapeHatchDummyLinter -> NoDummy and EscapeHatchNoNulls -> No…

086283d

…Null

Fix to handle fully qualified SuppressWarnings annotation

0d9d300

marcelocenerine force-pushed the suppress_warnings branch from 45596a8 to 7f8b021 Compare February 28, 2018 00:34

Address review comments

f6d9528

- Remove redundant expressions in AnnotationScopes test - Update docs to include example of mixing @SuppressWarnings and comments

marcelocenerine force-pushed the suppress_warnings branch from 7f8b021 to f6d9528 Compare February 28, 2018 00:43

olafurpg suggested changes Mar 6, 2018

View reviewed changes

marcelocenerine added 4 commits March 6, 2018 23:07

Eliminate subcategories of UnusedScalafixSuppression

4b6cdac

Adds Mods extractor in TreeExtractors

96feec0

Avoid crashing when rule name in @SuppressWarnings is not a literal

d65f7a7

Merge branch 'master' into suppress_warnings

a1ace99

marcelocenerine commented Mar 7, 2018

View reviewed changes

Change warning message for unused scalafix suppression

3f51c9d

MasseGuillaume approved these changes Mar 12, 2018

View reviewed changes

olafurpg approved these changes Mar 12, 2018

View reviewed changes

olafurpg merged commit be8cf71 into scalacenter:master Mar 12, 2018

olafurpg added this to the v0.6.0-M2 milestone Apr 11, 2018

This was referenced Apr 13, 2018

scalafix:ok disables scalafix:off #693

Closed

MissingFinal bugfixes #706

Merged

olafurpg mentioned this pull request May 2, 2018

Respect @SuppressWarnings to disable rules #442

Closed

marcelocenerine deleted the suppress_warnings branch November 23, 2023 16:43


		Note: Suppression via comments and `@SuppressWarnings` can be combined in the same source file. Be mindful not to
		introduce overlaps between the two as it can cause confusion. Scalafix gracefully handles overlaps by given


		It's possible to supress false positives linter message over source file regions using comments. This provides fine grained control over what to report
		## Suppressing rules

Respect @SuppressWarnings to disable rules #620

Respect @SuppressWarnings to disable rules #620

Conversation

marcelocenerine commented Feb 21, 2018 • edited Loading

Implementation details:

SuppressWarnings vs. @java.lang.SuppressWarnings

Refactoring

Improvement: Warnings to point to the exact position of unused rules

Bugs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg commented Feb 21, 2018

Choose a reason for hiding this comment

marcelocenerine Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MasseGuillaume Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MasseGuillaume commented Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcelocenerine Feb 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcelocenerine Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcelocenerine Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

marcelocenerine commented Mar 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcelocenerine commented Mar 12, 2018

marcelocenerine commented Feb 21, 2018 •

edited

Loading

`SuppressWarnings` vs. `@java.lang.SuppressWarnings`

marcelocenerine Feb 21, 2018 •

edited

Loading

MasseGuillaume Feb 21, 2018 •

edited

Loading

MasseGuillaume commented Feb 21, 2018 •

edited

Loading

marcelocenerine Feb 28, 2018 •

edited

Loading

marcelocenerine Mar 7, 2018 •

edited

Loading

marcelocenerine Mar 7, 2018 •

edited

Loading