From 93e4776c33001cdda1bb301ca5f66d6b84229c33 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 10 May 2024 10:17:11 +0200 Subject: [PATCH 01/23] Work on more rules --- .../main/scala/fix/IllegalFormatString.scala | 21 ++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/IllegalFormatString.scala | 39 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala new file mode 100644 index 0000000000..28d556b258 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala @@ -0,0 +1,21 @@ +/* +rule = IllegalFormatString + */ +package fix + +object IllegalFormatString { + def test(): Unit = { + val name = "John" + val age = 30 + val illegalFormatString1 = "%s is %d years old, %d" + println(illegalFormatString1.format(name, age)) // assert: IllegalFormatString + + val balance = 1000.50 + val illegalFormatString2 = "%2.2f CHF" + println(illegalFormatString2.format(balance)) // assert: IllegalFormatString + + val illegalFormatString3 = "Age: %s" + println(illegalFormatString3.format(age)) // assert: IllegalFormatString + + } +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index f5dfd9b08c..37ded39ea3 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -3,3 +3,4 @@ fix.CatchNpe fix.ComparingFloatingTypes fix.EmptyInterpolatedString fix.ImpossibleOptionSizeCondition +fix.IllegalFormatString diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala new file mode 100644 index 0000000000..391c92d41c --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -0,0 +1,39 @@ +/* +rule = IllegalFormatString + */ +package fix + +import scalafix.lint.LintSeverity + +import scala.meta._ +import scalafix.v1._ + +import java.util.{IllegalFormatException, MissingFormatArgumentException, UnknownFormatConversionException} + +case class IllegalFormatStringDiag(string: Tree) extends Diagnostic { + override def message: String = "Illegal format string" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "An unchecked exception will be thrown when a format string contains an illegal syntax or a format specifier that is incompatible with the given arguments" + + override def position: Position = string.pos +} + +class IllegalFormatString extends SemanticRule("IllegalFormatString") { + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(_, _)) => + println(qual.symbol.value) + //TODO see if worth implementing, since we would also have to handle cases: + // 1. String.format(format, args) + // 2. mystr.format(args) + // 3. "str".format(args) +// try String.format(format, args: _*) +// catch { +// case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => Patch.lint(IllegalFormatStringDiag(t)) +// } + Patch.empty + } + }.asPatch +} From 4f0121b05c0d40bdbb63bb921b084d0f88370aa8 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 17 May 2024 12:51:20 +0200 Subject: [PATCH 02/23] Built IllegalFormatString rule (still some input values left to add) and created Util file with util functions --- .../main/scala/fix/IllegalFormatString.scala | 14 +++--- .../scala/fix/ComparingFloatingTypes.scala | 11 +---- .../main/scala/fix/IllegalFormatString.scala | 48 ++++++++++++++----- .../fix/ImpossibleOptionSizeCondition.scala | 11 +---- .../rules/src/main/scala/fix/Util.scala | 42 ++++++++++++++++ 5 files changed, 88 insertions(+), 38 deletions(-) create mode 100644 be2-scala/linter/rules/src/main/scala/fix/Util.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala index 28d556b258..8735cafd36 100644 --- a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala @@ -5,17 +5,15 @@ package fix object IllegalFormatString { def test(): Unit = { - val name = "John" - val age = 30 + val name: String = "John" + val age: Integer = 30 val illegalFormatString1 = "%s is %d years old, %d" - println(illegalFormatString1.format(name, age)) // assert: IllegalFormatString + println(String.format(illegalFormatString1, name, age)) // assert: IllegalFormatString + - val balance = 1000.50 - val illegalFormatString2 = "%2.2f CHF" - println(illegalFormatString2.format(balance)) // assert: IllegalFormatString + println(illegalFormatString1.format(name, age)) // assert: IllegalFormatString - val illegalFormatString3 = "Age: %s" - println(illegalFormatString3.format(age)) // assert: IllegalFormatString + println("%s is %d years old, %d".format(name, age)) // assert: IllegalFormatString } } diff --git a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala index af99ed5925..238a645c37 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala @@ -3,6 +3,7 @@ rule = CatchNpe */ package fix +import fix.Util.getType import scalafix.lint.LintSeverity import scala.meta._ @@ -21,18 +22,10 @@ case class ComparingFloatingTypesDiag(floats: Tree) extends Diagnostic { class ComparingFloatingTypes extends SemanticRule("ComparingFloatingTypes") { override def fix(implicit doc: SemanticDocument): Patch = { - def getType(term: Term): Symbol = { - term.symbol.info match { - case Some(symInfo) => symInfo.signature match { - case ValueSignature(TypeRef(_, symbol, _)) => symbol - case _ => null - } - case _ => null - } - } def isFloatOrDouble(term: Term): Boolean = { val floatOrDoubleMatcher = SymbolMatcher.exact("scala/Float#", "scala/Double#") + val t = getType(term) floatOrDoubleMatcher.matches(getType(term)) } diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala index 391c92d41c..22875068ed 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -3,6 +3,7 @@ rule = IllegalFormatString */ package fix +import fix.Util.{findDefinition, findDefinitions, findDefinitionsOrdered} import scalafix.lint.LintSeverity import scala.meta._ @@ -22,18 +23,43 @@ case class IllegalFormatStringDiag(string: Tree) extends Diagnostic { class IllegalFormatString extends SemanticRule("IllegalFormatString") { override def fix(implicit doc: SemanticDocument): Patch = { + + //Term parameter is simply used to display the rule at the correct place + def rule(term: Term, value: String, args: List[Any]): Patch = { + try String.format(value, args.map(_.asInstanceOf[Object]): _*) + //cast is necessary since String.format() takes varargs of type Object and Any != Object + catch { + case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => return Patch.lint(IllegalFormatStringDiag(term)) + } + Patch.empty + } + + doc.tree.collect { - case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(_, _)) => - println(qual.symbol.value) - //TODO see if worth implementing, since we would also have to handle cases: - // 1. String.format(format, args) - // 2. mystr.format(args) - // 3. "str".format(args) -// try String.format(format, args: _*) -// catch { -// case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => Patch.lint(IllegalFormatStringDiag(t)) -// } - Patch.empty + case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, mod)) => + val mappedArgs = findDefinitionsOrdered(doc.tree, args) + qual match { + case Term.Name("String") => + //This case corresponds to String.format(format, args) + //String.format argclause has format string as first argument and rest are arguments, + // we can safely assume first argument is a string if we are doing a String.format() + rule(t, mappedArgs.head.toString, mappedArgs.tail) + case strName @ Term.Name(_) => + //This case corresponds to a string declared as a val or var and used in a format call, e.g. + // val myStr = "Hello %s" + // myStr.format("world") + val str = findDefinition(doc.tree, strName) + str match { + case value: String => rule(t, value, mappedArgs) + case _ => Patch.empty + } + case Lit.String(value) => + //This case corresponds to a string literal used in a format call, e.g. "Hello %s".format("world") + rule(t, value, mappedArgs) + case _ => Patch.empty + } } }.asPatch + + } diff --git a/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala b/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala index 021316aca8..0abf668b17 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala @@ -3,6 +3,7 @@ rule = ImpossibleOptionSizeCondition */ package fix +import fix.Util.getType import scalafix.lint.LintSeverity import scala.meta._ @@ -22,16 +23,6 @@ class ImpossibleOptionSizeCondition extends SemanticRule("ImpossibleOptionSizeCo override def fix(implicit doc: SemanticDocument): Patch = { - def getType(term: Term): Symbol = { - term.symbol.info match { - case Some(symInfo) => symInfo.signature match { - case ValueSignature(TypeRef(_, symbol, _)) => symbol - case _ => null - } - case _ => null - } - } - doc.tree.collect { case t @ Term.ApplyInfix.After_4_6_0(Term.Select(qual, Term.Name("size")), Term.Name(">") | Term.Name(">="), _, Term.ArgClause(List(comparedValue), _)) diff --git a/be2-scala/linter/rules/src/main/scala/fix/Util.scala b/be2-scala/linter/rules/src/main/scala/fix/Util.scala new file mode 100644 index 0000000000..5ae22e719f --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/Util.scala @@ -0,0 +1,42 @@ +package fix + +import scalafix.v1._ + +import scala.meta._ + +object Util { + def getType(term: Term)(implicit doc: SemanticDocument): Symbol = { + term.symbol.info match { + case Some(symInfo) => symInfo.signature match { + case ValueSignature(TypeRef(_, symbol, _)) => symbol + case _ => null + } + case _ => null + } + } + + def findDefinition(tree: Tree, name: Term): Any = { + tree.collect { + case Defn.Val(_, List(Pat.Var(varName)), _, Lit(value)) + if varName.value.equals(name.toString) => value + case Defn.Var.After_4_7_2(_, List(Pat.Var(varName)), _, value) + if varName == name => value + }.headOption.orNull + } + + def findDefinitions(tree: Tree, nameSet: Set[Term]): List[(Term, Any)] = { + tree.collect { + case Defn.Val(_, List(Pat.Var(varName)), _, Lit(value)) if nameSet.exists(_.toString().equals(varName.value)) => nameSet.find(_.toString().equals(varName.value)).get -> value + case Defn.Var.After_4_7_2(_, List(Pat.Var(varName)), _, value) if nameSet.exists(_.toString().equals(varName.value)) => nameSet.find(_.toString().equals(varName.value)).get -> value + } + } + + // Finds multiple definitions in one tree traversal + def findDefinitionsMap(tree: Tree, nameSet: Set[Term]): Map[Term, Any] = { + findDefinitions(tree, nameSet).toMap + } + + def findDefinitionsOrdered(tree: Tree, nameSet: List[Term]): List[Any] = { + findDefinitions(tree, nameSet.toSet).sortBy { case (term, _) => nameSet.indexOf(term) }.map { case (_, value) => value } + } +} From b4dbf951434995b6070aaeb2cad3a9e02f6cf8fe Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 17 May 2024 15:14:56 +0200 Subject: [PATCH 03/23] Added tests in format string rules, added support for literal.format() in EmptyInterpolatedString --- .../src/main/scala/fix/ArraysInFormat.scala | 5 +++ .../scala/fix/EmptyInterpolatedString.scala | 7 ++- .../main/scala/fix/IllegalFormatString.scala | 21 +++++++-- be2-scala/linter/project/TargetAxis.scala | 43 +++++++++++++++++++ .../scala/fix/EmptyInterpolatedString.scala | 2 +- .../main/scala/fix/IllegalFormatString.scala | 7 ++- 6 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 be2-scala/linter/project/TargetAxis.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/ArraysInFormat.scala b/be2-scala/linter/input/src/main/scala/fix/ArraysInFormat.scala index 62bb06201b..9e6f63a8d5 100644 --- a/be2-scala/linter/input/src/main/scala/fix/ArraysInFormat.scala +++ b/be2-scala/linter/input/src/main/scala/fix/ArraysInFormat.scala @@ -9,10 +9,15 @@ object ArraysInFormat { val str = f"Here are my cool elements ${array}" // assert: ArraysInFormat val str2 = s"Here are my cool elements ${array}" // assert: ArraysInFormat String.format("Here are my cool elements %d", array) // assert: ArraysInFormat + "Here are my cool elements %d".format(array) // assert: ArraysInFormat + val str3 = "Here are my cool elements %d" + str3.format(array) // assert: ArraysInFormat String.format("Here are my cool elements %d", array) /* assert: ArraysInFormat ^^^^^ Array passed to format / interpolate string */ + + "Here are my cool elements %d".format(13) // scalafix: ok; } } diff --git a/be2-scala/linter/input/src/main/scala/fix/EmptyInterpolatedString.scala b/be2-scala/linter/input/src/main/scala/fix/EmptyInterpolatedString.scala index 681086aff9..7f5646334d 100644 --- a/be2-scala/linter/input/src/main/scala/fix/EmptyInterpolatedString.scala +++ b/be2-scala/linter/input/src/main/scala/fix/EmptyInterpolatedString.scala @@ -6,7 +6,10 @@ package fix object EmptyInterpolatedString { def test(): Unit = { print(f"Here's my cute interpolation") // assert: EmptyInterpolatedString - print(s"Here's my amazing interpolationg") // assert: EmptyInterpolatedString - print(String.format("I'm hungry!")) // assert: EmptyInterpolatedString + print(s"Here's my amazing interpolation") // assert: EmptyInterpolatedString + String.format("I'm hungry!") // assert: EmptyInterpolatedString + val str = "I'm hungry!" + str.format() // assert: EmptyInterpolatedString + "I'm hungry!".format() // assert: EmptyInterpolatedString } } diff --git a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala index 8735cafd36..805a101f7d 100644 --- a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala @@ -8,12 +8,27 @@ object IllegalFormatString { val name: String = "John" val age: Integer = 30 val illegalFormatString1 = "%s is %d years old, %d" - println(String.format(illegalFormatString1, name, age)) // assert: IllegalFormatString + String.format(illegalFormatString1, name, age) // assert: IllegalFormatString + illegalFormatString1.format(name, age) // assert: IllegalFormatString - println(illegalFormatString1.format(name, age)) // assert: IllegalFormatString + "%s is %d years old, %d".format(name, age) // assert: IllegalFormatString + + "%5.5q".format("sam") // assert: IllegalFormatString + + "% s".format("sam") // assert: IllegalFormatString + + "%qs".format("sam") // assert: IllegalFormatString + + "%.-5s".format("sam") // assert: IllegalFormatString + + "%.s".format("sam") // assert: IllegalFormatString + + "% a.scalaVersion }.get + + /** When invoked on a ProjectMatrix with a TargetAxis, lookup the project generated by `matrix` with a scalaVersion matching the one declared in that TargetAxis, and resolve `key`. + */ + def resolve[T]( + matrix: ProjectMatrix, + key: TaskKey[T] + ): Def.Initialize[Task[T]] = + Def.taskDyn { + val sv = targetScalaVersion(virtualAxes.value) + val project = matrix.finder().apply(sv) + Def.task((project / key).value) + } + + /** When invoked on a ProjectMatrix with a TargetAxis, lookup the project generated by `matrix` with a scalaVersion matching the one declared in that TargetAxis, and resolve `key`. + */ + def resolve[T]( + matrix: ProjectMatrix, + key: SettingKey[T] + ): Def.Initialize[T] = + Def.settingDyn { + val sv = targetScalaVersion(virtualAxes.value) + val project = matrix.finder().apply(sv) + Def.setting((project / key).value) + } + +} diff --git a/be2-scala/linter/rules/src/main/scala/fix/EmptyInterpolatedString.scala b/be2-scala/linter/rules/src/main/scala/fix/EmptyInterpolatedString.scala index adb9a9dd2f..e722477660 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/EmptyInterpolatedString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/EmptyInterpolatedString.scala @@ -21,7 +21,7 @@ case class EmptyInterpolatedStringDiag(string: Tree) extends Diagnostic { class EmptyInterpolatedString extends SemanticRule("EmptyInterpolatedString") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case t @ Term.Apply.After_4_6_0(Term.Select(_, Term.Name("format")), Term.ArgClause(List(Lit.String(_)), _)) => Patch.lint(EmptyInterpolatedStringDiag(t)) + case t @ Term.Apply.After_4_6_0(Term.Select(_, Term.Name("format")), Term.ArgClause(List(Lit.String(_)), _) | Term.ArgClause(Nil, _)) => Patch.lint(EmptyInterpolatedStringDiag(t)) case t @ Term.Interpolate(_, _, Nil) => Patch.lint(EmptyInterpolatedStringDiag(t)) }.asPatch } diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala index 22875068ed..1723b52c3b 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -26,8 +26,7 @@ class IllegalFormatString extends SemanticRule("IllegalFormatString") { //Term parameter is simply used to display the rule at the correct place def rule(term: Term, value: String, args: List[Any]): Patch = { - try String.format(value, args.map(_.asInstanceOf[Object]): _*) - //cast is necessary since String.format() takes varargs of type Object and Any != Object + try value.format(args: _*) catch { case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => return Patch.lint(IllegalFormatStringDiag(term)) } @@ -36,8 +35,8 @@ class IllegalFormatString extends SemanticRule("IllegalFormatString") { doc.tree.collect { - case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, mod)) => - val mappedArgs = findDefinitionsOrdered(doc.tree, args) + case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, _)) => + val mappedArgs = findDefinitionsOrdered(doc.tree, args) ++ args.collect { case Lit(value) => value } //literals will not be found in the tree qual match { case Term.Name("String") => //This case corresponds to String.format(format, args) From ecfda686d2553303f9616b1a3f163c3e6e78e392 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 17 May 2024 16:29:09 +0200 Subject: [PATCH 04/23] Added IncorrectNumberOfArgsToFormat rule, reformatted other rules (converted nested methods to private class methods) --- .../main/scala/fix/IllegalFormatString.scala | 2 + .../fix/IncorrectNumberOfArgsToFormat.scala | 23 +++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../src/main/scala/fix/ArraysInFormat.scala | 23 +++---- .../scala/fix/ComparingFloatingTypes.scala | 1 - .../main/scala/fix/IllegalFormatString.scala | 33 +++++---- .../fix/IncorrectNumberOfArgsToFormat.scala | 67 +++++++++++++++++++ 7 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 be2-scala/linter/input/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala index 805a101f7d..d209d14033 100644 --- a/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/input/src/main/scala/fix/IllegalFormatString.scala @@ -29,6 +29,8 @@ object IllegalFormatString { "%.2f %s".format(14.5, "sammmmmmmmm") // scalafix: ok; "%010d".format(0) // scalafix: ok; + "%s is %d years old, %d".format(name, age, 145) // scalafix: ok; + } } diff --git a/be2-scala/linter/input/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala b/be2-scala/linter/input/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala new file mode 100644 index 0000000000..b13fe16015 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala @@ -0,0 +1,23 @@ +/* +rule = IncorrectNumberOfArgsToFormat + */ +package fix + +object IncorrectNumberOfArgsToFormat { + def test() = { + val name: String = "John" + val age: Integer = 30 + val illegalFormatString1 = "%s is %d years old, %d" + illegalFormatString1.format(name, age, 134, 54, "Olivia") // assert: IncorrectNumberOfArgsToFormat + illegalFormatString1.format(name) // assert: IncorrectNumberOfArgsToFormat + + "%s is %d years old, %d".format(name, age, 134, 54, "Olivia") // assert: IncorrectNumberOfArgsToFormat + "%s is %d years old, %d".format(name) // assert: IncorrectNumberOfArgsToFormat + + String.format(illegalFormatString1, name) // assert: IncorrectNumberOfArgsToFormat + + illegalFormatString1.format(name, age, 134) // scalafix: ok; + "%s is %d years old, %d".format(name, age, 134) // scalafix: ok; + } + +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 37ded39ea3..c57656d595 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -4,3 +4,4 @@ fix.ComparingFloatingTypes fix.EmptyInterpolatedString fix.ImpossibleOptionSizeCondition fix.IllegalFormatString +fix.IncorrectNumberOfArgsToFormat diff --git a/be2-scala/linter/rules/src/main/scala/fix/ArraysInFormat.scala b/be2-scala/linter/rules/src/main/scala/fix/ArraysInFormat.scala index ec7c74225b..b47c47a238 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ArraysInFormat.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ArraysInFormat.scala @@ -20,21 +20,20 @@ case class ArraysInFormatDiag(array: Tree) extends Diagnostic { class ArraysInFormat extends SemanticRule("ArraysInFormat") { - override def fix(implicit doc: SemanticDocument): Patch = { - - def rule(args: List[Stat]) = { - args.collect { - case t @ Term.Name(_) => - t.symbol.info match { - case Some(symInfo) => symInfo.signature match { - case ValueSignature(TypeRef(_, symbol, _)) if SymbolMatcher.exact("scala/Array#").matches(symbol) => Patch.lint(ArraysInFormatDiag(t)) - case _ => Patch.empty - } - case _ => Patch.empty + private def rule(args: List[Stat])(implicit doc: SemanticDocument) = { + args.collect { + case t @ Term.Name(_) => + t.symbol.info match { + case Some(symInfo) => symInfo.signature match { + case ValueSignature(TypeRef(_, symbol, _)) if SymbolMatcher.exact("scala/Array#").matches(symbol) => Patch.lint(ArraysInFormatDiag(t)) + case _ => Patch.empty } - } + case _ => Patch.empty + } } + } + override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { case Term.Apply.After_4_6_0(Term.Select(_, Term.Name("format")), Term.ArgClause(args, _)) => rule(args) case Term.Interpolate(_, _, args) => rule(args.flatMap { case Term.Block(stats) => stats }) // Args in Term.Interpolate come in Term.Block requiring to extract the stats diff --git a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala index 238a645c37..54fe8b6164 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala @@ -25,7 +25,6 @@ class ComparingFloatingTypes extends SemanticRule("ComparingFloatingTypes") { def isFloatOrDouble(term: Term): Boolean = { val floatOrDoubleMatcher = SymbolMatcher.exact("scala/Float#", "scala/Double#") - val t = getType(term) floatOrDoubleMatcher.matches(getType(term)) } diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala index 1723b52c3b..bd2653bddc 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -22,39 +22,48 @@ case class IllegalFormatStringDiag(string: Tree) extends Diagnostic { } class IllegalFormatString extends SemanticRule("IllegalFormatString") { - override def fix(implicit doc: SemanticDocument): Patch = { - //Term parameter is simply used to display the rule at the correct place - def rule(term: Term, value: String, args: List[Any]): Patch = { - try value.format(args: _*) - catch { - case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => return Patch.lint(IllegalFormatStringDiag(term)) - } - Patch.empty + //Term parameter is simply used to display the rule at the correct place + private def rule(term: Term, value: String, args: List[Any]): Patch = { + try value.format(args: _*) + catch { + case _: IllegalFormatException | _: MissingFormatArgumentException | _: UnknownFormatConversionException => return Patch.lint(IllegalFormatStringDiag(term)) } + Patch.empty + } + + override def fix(implicit doc: SemanticDocument): Patch = { + def getMappedArgs(args: List[Term]): List[Any] = { + findDefinitionsOrdered(doc.tree, args) ++ args.collect { case Lit(value) => value } + } doc.tree.collect { case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, _)) => - val mappedArgs = findDefinitionsOrdered(doc.tree, args) ++ args.collect { case Lit(value) => value } //literals will not be found in the tree qual match { case Term.Name("String") => //This case corresponds to String.format(format, args) //String.format argclause has format string as first argument and rest are arguments, // we can safely assume first argument is a string if we are doing a String.format() - rule(t, mappedArgs.head.toString, mappedArgs.tail) + args match { + case Lit.String(format) :: rest => rule(t, format, getMappedArgs(rest)) + case Term.Name(_) :: _ => + val mappedArgs = getMappedArgs(args) + rule(t, mappedArgs.head.toString, mappedArgs.tail) + case _ => Patch.empty + } case strName @ Term.Name(_) => //This case corresponds to a string declared as a val or var and used in a format call, e.g. // val myStr = "Hello %s" // myStr.format("world") val str = findDefinition(doc.tree, strName) str match { - case value: String => rule(t, value, mappedArgs) + case value: String => rule(t, value, getMappedArgs(args)) case _ => Patch.empty } case Lit.String(value) => //This case corresponds to a string literal used in a format call, e.g. "Hello %s".format("world") - rule(t, value, mappedArgs) + rule(t, value, getMappedArgs(args)) case _ => Patch.empty } } diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala new file mode 100644 index 0000000000..d9fc454cd7 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala @@ -0,0 +1,67 @@ +/* +rule = IncorrectNumberOfArgsToFormat + */ +package fix + +import fix.Util.findDefinition +import scalafix.lint.LintSeverity +import scalafix.v1._ + +import scala.meta._ + +case class IncorrectNumberOfArgsToFormatDiag(string: Tree) extends Diagnostic { + override def message: String = "Incorrect number of arguments to format" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "The number of arguments passed to String.format doesn't correspond to the number of fields in the format string." + + override def position: Position = string.pos +} + +class IncorrectNumberOfArgsToFormat extends SemanticRule("IncorrectNumberOfArgsToFormat") { + + private val argRegex = "%((\\d+\\$)?[-#+ 0,(\\<]*?\\d?(\\.\\d+)?[tT]?[a-zA-Z]|%)".r + + private def rule(format: String, args: List[Any], t: Term): Patch = { + val argCount = argRegex + .findAllIn(format) + .matchData + .count(m => !doesNotTakeArguments(m.matched)) + + if (argCount != args.size) Patch.lint(IncorrectNumberOfArgsToFormatDiag(t)) else Patch.empty //TODO test with != + } + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, _)) => + qual match { + case Term.Name("String") => + args match { + case Lit.String(format) :: rest => rule(format, rest, t) + case (head @ Term.Name(_)) :: rest => + val format = findDefinition(doc.tree, head) + format match { + case value: String => rule(value, rest, t) + } + case _ => Patch.empty + } + val format = findDefinition(doc.tree,args.head) + format match { + case value: String => rule(value, args.tail, t) + case _ => Patch.empty + } + case strName @ Term.Name(_) => val str = findDefinition(doc.tree, strName) + str match { + case value: String => rule(value, args, t) + case _ => Patch.empty + } + case Lit.String(value) => rule(value, args, t) + + } + }.asPatch + } + + private def doesNotTakeArguments(formatSpecifier: String): Boolean = + formatSpecifier == "%%" || formatSpecifier == "%n" +} From 2d3de5e90cfe7ac261d2f731cbb6f9300db7b62a Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 17 May 2024 16:29:34 +0200 Subject: [PATCH 05/23] Added IncorrectNumberOfArgsToFormat rule, reformatted other rules (converted nested methods to private class methods) --- .../src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala index d9fc454cd7..6a0e559fdf 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala @@ -29,7 +29,7 @@ class IncorrectNumberOfArgsToFormat extends SemanticRule("IncorrectNumberOfArgsT .matchData .count(m => !doesNotTakeArguments(m.matched)) - if (argCount != args.size) Patch.lint(IncorrectNumberOfArgsToFormatDiag(t)) else Patch.empty //TODO test with != + if (argCount != args.size) Patch.lint(IncorrectNumberOfArgsToFormatDiag(t)) else Patch.empty } override def fix(implicit doc: SemanticDocument): Patch = { From dc9cf71945be817c4f2267e8f0fd25d04b99342d Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 22 May 2024 15:51:47 +0200 Subject: [PATCH 06/23] Implemented IncorrectlyNamedExceptions --- .../fix/IncorrectlyNamedExceptions.scala | 24 ++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../fix/IncorrectlyNamedExceptions.scala | 59 +++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala b/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala new file mode 100644 index 0000000000..b6e281efad --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala @@ -0,0 +1,24 @@ +/* +rule = IncorrectlyNamedExceptions + */ +package fix + +object IncorrectlyNamedExceptions { + class NotException // scalafix: ok; + + class IsException extends Exception // scalafix: ok; + + class IsNotNamedCorrectly extends Exception // assert: IncorrectlyNamedExceptions + + class Is extends Exception // assert: IncorrectlyNamedExceptions + + class IsChild extends Is // assert: IncorrectlyNamedExceptions + + class IsChildException // scalafix: ok; + + class IsRuntimeException extends RuntimeException // scalafix: ok; + + class IsRuntime extends Exception // assert: IncorrectlyNamedExceptions + +} + diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index c57656d595..102b386328 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -5,3 +5,4 @@ fix.EmptyInterpolatedString fix.ImpossibleOptionSizeCondition fix.IllegalFormatString fix.IncorrectNumberOfArgsToFormat +fix.IncorrectlyNamedExceptions diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala new file mode 100644 index 0000000000..6ff387f60f --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala @@ -0,0 +1,59 @@ +/* +rule = IncorrectlyNamedExceptions + */ +package fix + +import scalafix.lint.LintSeverity + +import scala.meta._ +import scalafix.v1._ + +case class IncorrectlyNamedExceptionsDiag(exception: Tree) extends Diagnostic { + override def message: String = "Incorrectly named exceptions" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Class named exception does not derive from Exception / class derived from Exception is not named *Exception." + + override def position: Position = exception.pos +} + +class IncorrectlyNamedExceptions extends SemanticRule("IncorrectlyNamedExceptions") { + + private def inheritsFromException(symbol: Symbol)(implicit doc: SemanticDocument): Boolean = { + symbol.info match { + case Some(info) => + // Check if the current type contains an Exception in its name + info.signature match { + case ClassSignature(_, _, _, _) if info.displayName.contains("Exception") => true + case ClassSignature(_, parents, _, _) => + // Recursively check parent types + parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException(_)) + case TypeSignature(_, TypeRef(_, symbol1, _), TypeRef(_, symbol2, _)) => + // Check if inherits java.lang.Exception + val matcher = SymbolMatcher.exact("java/lang/Exception#") + matcher.matches(symbol1) || matcher.matches(symbol2) + case _ => false + } + case None => false + } + } + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case cl@Defn.Class.After_4_6_0(_, Type.Name(name), _, _, _) => + cl.symbol.info.get.signature match { + case ClassSignature(_, parents, _, _) => + if (!name.contains("Exception") && (parents.exists(p => p.toString.contains("Exception")) + || parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException))) { + Patch.lint(IncorrectlyNamedExceptionsDiag(cl)) + } else { + Patch.empty + + } + case _ => Patch.empty + } + }.asPatch + } + +} From 8c562cb6aa3d350ce4eafd35da01cc6091c43135 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 22 May 2024 15:53:42 +0200 Subject: [PATCH 07/23] Simplified test --- .../rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala index 6ff387f60f..f27c198dbe 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala @@ -44,8 +44,7 @@ class IncorrectlyNamedExceptions extends SemanticRule("IncorrectlyNamedException case cl@Defn.Class.After_4_6_0(_, Type.Name(name), _, _, _) => cl.symbol.info.get.signature match { case ClassSignature(_, parents, _, _) => - if (!name.contains("Exception") && (parents.exists(p => p.toString.contains("Exception")) - || parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException))) { + if (!name.contains("Exception") && parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException)) { Patch.lint(IncorrectlyNamedExceptionsDiag(cl)) } else { Patch.empty From a495c08b0cc5c7568511c663f8181c33fd7dfafc Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 22 May 2024 15:54:51 +0200 Subject: [PATCH 08/23] Cleaned-up code --- .../src/main/scala/fix/IncorrectlyNamedExceptions.scala | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala index f27c198dbe..a2eaf1e8cf 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectlyNamedExceptions.scala @@ -41,15 +41,12 @@ class IncorrectlyNamedExceptions extends SemanticRule("IncorrectlyNamedException override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case cl@Defn.Class.After_4_6_0(_, Type.Name(name), _, _, _) => + case cl @ Defn.Class.After_4_6_0(_, Type.Name(name), _, _, _) => cl.symbol.info.get.signature match { case ClassSignature(_, parents, _, _) => - if (!name.contains("Exception") && parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException)) { + if (!name.contains("Exception") && parents.map(_.asInstanceOf[TypeRef].symbol).exists(inheritsFromException)) Patch.lint(IncorrectlyNamedExceptionsDiag(cl)) - } else { - Patch.empty - - } + else Patch.empty case _ => Patch.empty } }.asPatch From e6b67f95285ed93d3cf3d3d9f411c13cc5a48440 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 23 May 2024 18:10:04 +0200 Subject: [PATCH 09/23] Implemented LonelySealedTrait --- .../main/scala/fix/LonelySealedTrait.scala | 105 ++++++++++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/LonelySealedTrait.scala | 72 ++++++++++++ 3 files changed, 178 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala new file mode 100644 index 0000000000..ae2a75e6a6 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala @@ -0,0 +1,105 @@ +/* +rule = LonelySealedTrait + */ +package fix + +object LonelySealedTrait { + + sealed trait NoImpl // assert: LonelySealedTrait + + sealed trait Impl1 // scalafix: ok; + object Implemented extends Impl1 + + sealed trait Impl2 // scalafix: ok; + case object Implemented2 extends Impl2 + + sealed trait Impl3 // scalafix: ok; + case object Implemented3 extends Serializable with Impl3 + + sealed trait Impl4 // scalafix: ok; + case class Implemented4() extends Impl4 + + sealed trait Impl5 // scalafix: ok; + class Implemented extends Impl5 + + sealed trait Impl6 // scalafix: ok; + case class Implemented6(name: String) extends Serializable with Impl6 + + sealed trait Impl7 // scalafix: ok; + class Implemented7(name: String) extends Serializable with Impl7 + + sealed trait Impl8 // scalafix: ok; + case class Implemented8(name: String) extends Impl8 + + sealed trait Impl9 // scalafix: ok; + class Implemented9(name: String) extends Impl9 + + sealed trait Impl10 // scalafix: ok; + sealed trait Impl11 // scalafix: ok; + case class Implemented10(name: String) extends Impl10 + case class Implemented11(name: String) extends Impl11 + + trait AnalyzerFilter { + def name: String + } + + trait AnalyzerFilterDefinition { + def filterType: String + } + + sealed trait CharFilter extends AnalyzerFilter // scalafix: ok; + + sealed trait CharFilterDefinition extends CharFilter with AnalyzerFilterDefinition // scalafix: ok; + + case object HtmlStripCharFilter extends CharFilter { + val name = "html_strip" + } + + case class MappingCharFilter(name: String, mappings: (String, String)*) + extends CharFilterDefinition { + val filterType = "mapping" + } + + case class PatternReplaceCharFilter(name: String, pattern: String, replacement: String) + extends CharFilterDefinition { + val filterType = "pattern_replace" + } + + sealed abstract class MultiMode(val elastic: String) // scalafix: ok; + case object MultiMode { + case object Min extends MultiMode("min") + case object Max extends MultiMode("max") + case object Sum extends MultiMode("sum") + case object Avg extends MultiMode("avg") + } + + sealed trait A[S] // scalafix: ok; + case object B extends A[String] + case object C extends A[BigDecimal] + + sealed abstract class IndexOptionsNoImplementation(val value: String) // assert: LonelySealedTrait + + sealed abstract class IndexOptions(val value: String) // scalafix: ok; + object IndexOptions { + case object Docs extends IndexOptions("docs") + case object Freqs extends IndexOptions("freqs") +// case object Positions extends IndexOptions("positions") + } + + sealed trait D // scalafix: ok; + sealed trait E extends D + sealed trait F extends E + object F1 extends F + + sealed class G // scalafix: ok; + sealed class H extends G + sealed class I extends H + object I1 extends I + + sealed trait J // scalafix: ok; + sealed trait K extends J + sealed class L extends K + object L1 extends L + + +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 102b386328..8c403247c7 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -6,3 +6,4 @@ fix.ImpossibleOptionSizeCondition fix.IllegalFormatString fix.IncorrectNumberOfArgsToFormat fix.IncorrectlyNamedExceptions +fix.LonelySealedTrait diff --git a/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala b/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala new file mode 100644 index 0000000000..4d54a72053 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala @@ -0,0 +1,72 @@ +/* +rule = LonelySealedTrait + */ +package fix + +import scalafix.lint.LintSeverity + +import scala.meta._ +import scalafix.v1._ + +import scala.collection.mutable + +case class LonelySealedTraitDiag(t: Tree) extends Diagnostic { + override def message: String = "Lonely sealed trait" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "A sealed trait that is not extended is considered dead code." + + override def position: Position = t.pos +} + +class LonelySealedTrait extends SemanticRule("LonelySealedTrait") { + + private def findBaseClasses(parent: String, sealedTraitsHierarchy: mutable.Map[String, Set[String]]): Set[String] = { + sealedTraitsHierarchy.get(parent) match { + case Some(baseClasses) => + baseClasses ++ baseClasses.flatMap(findBaseClasses(_, sealedTraitsHierarchy)) + case None => + Set.empty[String] + } + } + + + override def fix(implicit doc: SemanticDocument): Patch = { + val sealedTraits = mutable.Map[String, Defn]() + val parents = mutable.Set[String]() + val sealedTraitsHierarchy = mutable.Map[String, Set[String]]() + + + // Use display name to handle the case A[B] where A is a trait and B is a type parameter + def prettyInits(inits: List[Init]): List[String] = inits.collect { case i if i.symbol.info.isDefined => i.symbol.info.get.displayName } + + def handleSealedTrait(inits: List[Init], cl: Defn, clname: String): Unit = { + if(inits.isEmpty) sealedTraitsHierarchy += (clname -> Set()) + else { + prettyInits(inits).foreach(i => sealedTraitsHierarchy += (i -> (sealedTraitsHierarchy.getOrElse(i, Set()) + clname))) + } + sealedTraits += (clname -> cl) + } + + + doc.tree.traverse { + case tr @ Defn.Trait.After_4_6_0(mods, name, _, _, Template.After_4_4_0(_, inits, _, _, _)) + if mods.exists(m => m.toString.equals("sealed")) => handleSealedTrait(inits, tr, name.value) + case cls @ Defn.Class.After_4_6_0(mods, name, _, _, Template.After_4_4_0(_, inits, _, _, _)) + if mods.exists(m => m.toString.equals("sealed")) => handleSealedTrait(inits, cls, name.value) + case cl @ Defn.Class.After_4_6_0(_, _, _, _, _) => cl.symbol.info.get.signature match { + case ClassSignature(_, pars, _, _) => pars.foreach(p => parents += p.toString()) + case _ => () // Class should have class signature + } + case Defn.Object(_, _, Template.After_4_4_0(_, inits, _, _, _)) => parents ++= prettyInits(inits) + } + + sealedTraits.collect({ + case (name, cl) if !parents.contains(name) + && parents.intersect(findBaseClasses(name,sealedTraitsHierarchy)).isEmpty => Patch.lint(LonelySealedTraitDiag(cl)) + //Either this sealed trait is directly extended or one of its sealed base classes / traits is being extended + case _ => Patch.empty + }).asPatch + } +} From 80c99a7836bc95384b109c1652d144f50c8ce82f Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 23 May 2024 18:10:04 +0200 Subject: [PATCH 10/23] Implemented LonelySealedTrait --- .../main/scala/fix/LonelySealedTrait.scala | 105 ++++++++++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/LonelySealedTrait.scala | 72 ++++++++++++ 3 files changed, 178 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala new file mode 100644 index 0000000000..5f8b85334a --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala @@ -0,0 +1,105 @@ +/* +rule = LonelySealedTrait + */ +package fix + +object LonelySealedTrait { + + sealed trait NoImpl // assert: LonelySealedTrait + + sealed trait Impl1 // scalafix: ok; + object Implemented extends Impl1 + + sealed trait Impl2 // scalafix: ok; + case object Implemented2 extends Impl2 + + sealed trait Impl3 // scalafix: ok; + case object Implemented3 extends Serializable with Impl3 + + sealed trait Impl4 // scalafix: ok; + case class Implemented4() extends Impl4 + + sealed trait Impl5 // scalafix: ok; + class Implemented extends Impl5 + + sealed trait Impl6 // scalafix: ok; + case class Implemented6(name: String) extends Serializable with Impl6 + + sealed trait Impl7 // scalafix: ok; + class Implemented7(name: String) extends Serializable with Impl7 + + sealed trait Impl8 // scalafix: ok; + case class Implemented8(name: String) extends Impl8 + + sealed trait Impl9 // scalafix: ok; + class Implemented9(name: String) extends Impl9 + + sealed trait Impl10 // scalafix: ok; + sealed trait Impl11 // scalafix: ok; + case class Implemented10(name: String) extends Impl10 + case class Implemented11(name: String) extends Impl11 + + trait AnalyzerFilter { + def name: String + } + + trait AnalyzerFilterDefinition { + def filterType: String + } + + sealed trait CharFilter extends AnalyzerFilter // scalafix: ok; + + sealed trait CharFilterDefinition extends CharFilter with AnalyzerFilterDefinition // scalafix: ok; + + case object HtmlStripCharFilter extends CharFilter { + val name = "html_strip" + } + + case class MappingCharFilter(name: String, mappings: (String, String)*) + extends CharFilterDefinition { + val filterType = "mapping" + } + + case class PatternReplaceCharFilter(name: String, pattern: String, replacement: String) + extends CharFilterDefinition { + val filterType = "pattern_replace" + } + + sealed abstract class MultiMode(val elastic: String) // scalafix: ok; + case object MultiMode { + case object Min extends MultiMode("min") + case object Max extends MultiMode("max") + case object Sum extends MultiMode("sum") + case object Avg extends MultiMode("avg") + } + + sealed trait A[S] // scalafix: ok; + case object B extends A[String] + case object C extends A[BigDecimal] + + sealed abstract class IndexOptionsNoImplementation(val value: String) // assert: LonelySealedTrait + + sealed abstract class IndexOptions(val value: String) // scalafix: ok; + object IndexOptions { + case object Docs extends IndexOptions("docs") + case object Freqs extends IndexOptions("freqs") + case object Positions extends IndexOptions("positions") + } + + sealed trait D // scalafix: ok; + sealed trait E extends D + sealed trait F extends E + object F1 extends F + + sealed class G // scalafix: ok; + sealed class H extends G + sealed class I extends H + object I1 extends I + + sealed trait J // scalafix: ok; + sealed trait K extends J + sealed class L extends K + object L1 extends L + + +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 102b386328..8c403247c7 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -6,3 +6,4 @@ fix.ImpossibleOptionSizeCondition fix.IllegalFormatString fix.IncorrectNumberOfArgsToFormat fix.IncorrectlyNamedExceptions +fix.LonelySealedTrait diff --git a/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala b/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala new file mode 100644 index 0000000000..4d54a72053 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/LonelySealedTrait.scala @@ -0,0 +1,72 @@ +/* +rule = LonelySealedTrait + */ +package fix + +import scalafix.lint.LintSeverity + +import scala.meta._ +import scalafix.v1._ + +import scala.collection.mutable + +case class LonelySealedTraitDiag(t: Tree) extends Diagnostic { + override def message: String = "Lonely sealed trait" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "A sealed trait that is not extended is considered dead code." + + override def position: Position = t.pos +} + +class LonelySealedTrait extends SemanticRule("LonelySealedTrait") { + + private def findBaseClasses(parent: String, sealedTraitsHierarchy: mutable.Map[String, Set[String]]): Set[String] = { + sealedTraitsHierarchy.get(parent) match { + case Some(baseClasses) => + baseClasses ++ baseClasses.flatMap(findBaseClasses(_, sealedTraitsHierarchy)) + case None => + Set.empty[String] + } + } + + + override def fix(implicit doc: SemanticDocument): Patch = { + val sealedTraits = mutable.Map[String, Defn]() + val parents = mutable.Set[String]() + val sealedTraitsHierarchy = mutable.Map[String, Set[String]]() + + + // Use display name to handle the case A[B] where A is a trait and B is a type parameter + def prettyInits(inits: List[Init]): List[String] = inits.collect { case i if i.symbol.info.isDefined => i.symbol.info.get.displayName } + + def handleSealedTrait(inits: List[Init], cl: Defn, clname: String): Unit = { + if(inits.isEmpty) sealedTraitsHierarchy += (clname -> Set()) + else { + prettyInits(inits).foreach(i => sealedTraitsHierarchy += (i -> (sealedTraitsHierarchy.getOrElse(i, Set()) + clname))) + } + sealedTraits += (clname -> cl) + } + + + doc.tree.traverse { + case tr @ Defn.Trait.After_4_6_0(mods, name, _, _, Template.After_4_4_0(_, inits, _, _, _)) + if mods.exists(m => m.toString.equals("sealed")) => handleSealedTrait(inits, tr, name.value) + case cls @ Defn.Class.After_4_6_0(mods, name, _, _, Template.After_4_4_0(_, inits, _, _, _)) + if mods.exists(m => m.toString.equals("sealed")) => handleSealedTrait(inits, cls, name.value) + case cl @ Defn.Class.After_4_6_0(_, _, _, _, _) => cl.symbol.info.get.signature match { + case ClassSignature(_, pars, _, _) => pars.foreach(p => parents += p.toString()) + case _ => () // Class should have class signature + } + case Defn.Object(_, _, Template.After_4_4_0(_, inits, _, _, _)) => parents ++= prettyInits(inits) + } + + sealedTraits.collect({ + case (name, cl) if !parents.contains(name) + && parents.intersect(findBaseClasses(name,sealedTraitsHierarchy)).isEmpty => Patch.lint(LonelySealedTraitDiag(cl)) + //Either this sealed trait is directly extended or one of its sealed base classes / traits is being extended + case _ => Patch.empty + }).asPatch + } +} From 0680e5fdb73134e9f42cc0dc8fd3a34ff88a75cb Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 23 May 2024 18:12:56 +0200 Subject: [PATCH 11/23] Added comments --- .../input/src/main/scala/fix/IncorrectlyNamedExceptions.scala | 1 + .../linter/input/src/main/scala/fix/LonelySealedTrait.scala | 1 + 2 files changed, 2 insertions(+) diff --git a/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala b/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala index b6e281efad..eaa620ff53 100644 --- a/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala +++ b/be2-scala/linter/input/src/main/scala/fix/IncorrectlyNamedExceptions.scala @@ -3,6 +3,7 @@ rule = IncorrectlyNamedExceptions */ package fix +// Test values taken from Scapegoat test object IncorrectlyNamedExceptions { class NotException // scalafix: ok; diff --git a/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala index 5f8b85334a..8b177c6a0e 100644 --- a/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala +++ b/be2-scala/linter/input/src/main/scala/fix/LonelySealedTrait.scala @@ -3,6 +3,7 @@ rule = LonelySealedTrait */ package fix +// Test values taken from Scapegoat test object LonelySealedTrait { sealed trait NoImpl // assert: LonelySealedTrait From d5f9381b8d30cfba48f14f4c2d74a252738433b6 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 23 May 2024 19:20:45 +0200 Subject: [PATCH 12/23] Added MapGetAndGetOrElse --- .../main/scala/fix/MapGetAndGetOrElse.scala | 19 ++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/MapGetAndGetOrElse.scala | 37 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/MapGetAndGetOrElse.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/MapGetAndGetOrElse.scala b/be2-scala/linter/input/src/main/scala/fix/MapGetAndGetOrElse.scala new file mode 100644 index 0000000000..32c66861cf --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/MapGetAndGetOrElse.scala @@ -0,0 +1,19 @@ +/* +rule = MapGetAndGetOrElse + */ +package fix + +object MapGetAndGetOrElse { + def test() = { + val numMap1 = Map(1 -> "one", 2 -> "two") + numMap1.get(1).getOrElse("unknown") // assert: MapGetAndGetOrElse + + + val numMap2 = scala.collection.mutable.Map("one" -> 1, "two" -> 2) + numMap2.get("one").getOrElse(-1) // assert: MapGetAndGetOrElse + + Map("John" -> "Smith", "Peter" -> "Rabbit").get("Sarah").getOrElse("-") // assert: MapGetAndGetOrElse + + } + +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 8c403247c7..de319c378f 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -7,3 +7,4 @@ fix.IllegalFormatString fix.IncorrectNumberOfArgsToFormat fix.IncorrectlyNamedExceptions fix.LonelySealedTrait +fix.MapGetAndGetOrElse diff --git a/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala b/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala new file mode 100644 index 0000000000..c1102fda9c --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala @@ -0,0 +1,37 @@ +/* +rule = MapGetAndGetOrElse + */ +package fix + +import scalafix.lint.LintSeverity + +import scala.meta._ +import scalafix.v1._ + +case class MapGetAndGetOrElseDiag(map: Tree) extends Diagnostic { + override def message: String = "Using of Map.get().getOrElse instead of Map.getOrElse()" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Map.get(key).getOrElse(value) can be replaced with Map.getOrElse(key, value), which is more concise." + + override def position: Position = map.pos +} + +class MapGetAndGetOrElse extends SemanticRule("MapGetAndGetOrElse") { + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case Term.Apply.After_4_6_0(Term.Select( + Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("get")), _), Term.Name("getOrElse")), _) => + qual match { + case variable @ Term.Name(_) + if SymbolMatcher.normalized("scala/collection/immutable/Map#", "scala/collection/mutable/Map#") + .matches(Util.getType(variable)) => Patch.lint(MapGetAndGetOrElseDiag(variable)) + case Term.Apply.After_4_6_0(Term.Name("Map"), _) => Patch.lint(MapGetAndGetOrElseDiag(qual)) + case _ => Patch.empty + } + case _ => Patch.empty + }.asPatch + } +} From 65c12bfc9917cc3616d56b69f0a4b2f2a84532f7 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 29 May 2024 10:46:13 +0200 Subject: [PATCH 13/23] Added NanComparison rule Corrected functionality in findDefinition (could previously only find literal definitions). Adapted other rules (IncorrectNumberOfArgsToFormat, IllegalFormatString) to modification --- .../src/main/scala/fix/NanComparison.scala | 25 +++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/IllegalFormatString.scala | 2 +- .../fix/IncorrectNumberOfArgsToFormat.scala | 6 +-- .../src/main/scala/fix/NanComparison.scala | 43 +++++++++++++++++++ .../rules/src/main/scala/fix/Util.scala | 2 +- 6 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 be2-scala/linter/input/src/main/scala/fix/NanComparison.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala b/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala new file mode 100644 index 0000000000..eed0bf7c72 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala @@ -0,0 +1,25 @@ +/* +rule = NanComparison + */ +package fix + +object NanComparison { + + def test() = { + val d = 0.5d + d == Double.NaN // assert: NanComparison + Double.NaN == d // assert: NanComparison + + val f = 0.5f + f == Double.NaN // assert: NanComparison + Double.NaN == f // assert: NanComparison + + val g = Double.NaN + f == g // assert: NanComparison + g == f // assert: NanComparison + + d == g // assert: NanComparison + g == d // assert: NanComparison + } + +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index de319c378f..37c5025163 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -8,3 +8,4 @@ fix.IncorrectNumberOfArgsToFormat fix.IncorrectlyNamedExceptions fix.LonelySealedTrait fix.MapGetAndGetOrElse +fix.NanComparison diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala index bd2653bddc..4f8cc8a17f 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -58,7 +58,7 @@ class IllegalFormatString extends SemanticRule("IllegalFormatString") { // myStr.format("world") val str = findDefinition(doc.tree, strName) str match { - case value: String => rule(t, value, getMappedArgs(args)) + case Lit.String(value) => rule(t, value, getMappedArgs(args)) case _ => Patch.empty } case Lit.String(value) => diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala index 6a0e559fdf..735a096893 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala @@ -42,18 +42,18 @@ class IncorrectNumberOfArgsToFormat extends SemanticRule("IncorrectNumberOfArgsT case (head @ Term.Name(_)) :: rest => val format = findDefinition(doc.tree, head) format match { - case value: String => rule(value, rest, t) + case Lit.String(value) => rule(value, rest, t) } case _ => Patch.empty } val format = findDefinition(doc.tree,args.head) format match { - case value: String => rule(value, args.tail, t) + case Lit.String(value) => rule(value, args.tail, t) case _ => Patch.empty } case strName @ Term.Name(_) => val str = findDefinition(doc.tree, strName) str match { - case value: String => rule(value, args, t) + case Lit.String(value) => rule(value, args, t) case _ => Patch.empty } case Lit.String(value) => rule(value, args, t) diff --git a/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala new file mode 100644 index 0000000000..64ed0bb0f8 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala @@ -0,0 +1,43 @@ +/* +rule = NanComparison +*/ +package fix + +import scalafix.lint.LintSeverity +import scalafix.v1._ +import scala.meta._ + +case class NanComparisonDiag(tree: Tree) extends Diagnostic { + override def message: String = "Comparison with NaN" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "NaN comparison will always fail. Use value.isNaN instead." + + override def position: Position = tree.pos +} + +class NanComparison extends SemanticRule("NanComparison") { + + override def fix(implicit doc: SemanticDocument): Patch = { + + def matcher(l: Any, r: Any, t: Tree): Patch = { + // Either we have definitions to extract or we have direct terms and matching is the same + // => matcher function avoids repetition. Type of arguments is any since findDefinition will return Any, + // but it doesn't matter since we do pattern matching + (l, r) match { + case (Term.Select(Term.Name("Double"), Term.Name("NaN")), _) | (_, Term.Select(Term.Name("Double"), Term.Name("NaN"))) => Patch.lint(NanComparisonDiag(t)) + case _ => Patch.empty + } + } + + doc.tree.collect { + case t @ Term.ApplyInfix.After_4_6_0(lhs, Term.Name("=="), _, Term.ArgClause(List(rhs), _)) => + (lhs,rhs) match { + case (Term.Name(_), Term.Name(_)) => matcher(Util.findDefinition(doc.tree, lhs), Util.findDefinition(doc.tree, rhs), t) + // Extract definitions and match in the case both are variables + case _ => matcher(lhs, rhs, t) + } + }.asPatch + } +} diff --git a/be2-scala/linter/rules/src/main/scala/fix/Util.scala b/be2-scala/linter/rules/src/main/scala/fix/Util.scala index 5ae22e719f..9cd8796449 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/Util.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/Util.scala @@ -17,7 +17,7 @@ object Util { def findDefinition(tree: Tree, name: Term): Any = { tree.collect { - case Defn.Val(_, List(Pat.Var(varName)), _, Lit(value)) + case Defn.Val(_, List(Pat.Var(varName)), _, value) if varName.value.equals(name.toString) => value case Defn.Var.After_4_7_2(_, List(Pat.Var(varName)), _, value) if varName == name => value From ecbe196dfc9f337cc0ac5054e627cfd1ab70e16c Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 29 May 2024 10:51:53 +0200 Subject: [PATCH 14/23] Removed useless code in IncorrectNumberOfArgsToFormat --- .../fix/IncorrectNumberOfArgsToFormat.scala | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala index 735a096893..b617d6cda2 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IncorrectNumberOfArgsToFormat.scala @@ -37,18 +37,13 @@ class IncorrectNumberOfArgsToFormat extends SemanticRule("IncorrectNumberOfArgsT case t @ Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("format")), Term.ArgClause(args, _)) => qual match { case Term.Name("String") => - args match { - case Lit.String(format) :: rest => rule(format, rest, t) - case (head @ Term.Name(_)) :: rest => - val format = findDefinition(doc.tree, head) - format match { - case Lit.String(value) => rule(value, rest, t) - } - case _ => Patch.empty - } - val format = findDefinition(doc.tree,args.head) - format match { - case Lit.String(value) => rule(value, args.tail, t) + args match { + case Lit.String(format) :: rest => rule(format, rest, t) + case (head @ Term.Name(_)) :: rest => + val format = findDefinition(doc.tree, head) + format match { + case Lit.String(value) => rule(value, rest, t) + } case _ => Patch.empty } case strName @ Term.Name(_) => val str = findDefinition(doc.tree, strName) @@ -59,6 +54,7 @@ class IncorrectNumberOfArgsToFormat extends SemanticRule("IncorrectNumberOfArgsT case Lit.String(value) => rule(value, args, t) } + case _ => Patch.empty }.asPatch } From e4aa58b9f456598f160c733b1a9eb153883e6431 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 29 May 2024 11:26:19 +0200 Subject: [PATCH 15/23] Added OptionGet and OptionSize --- .../input/src/main/scala/fix/OptionGet.scala | 13 +++++++ .../input/src/main/scala/fix/OptionSize.scala | 13 +++++++ .../META-INF/services/scalafix.v1.Rule | 2 ++ .../rules/src/main/scala/fix/OptionGet.scala | 34 +++++++++++++++++++ .../rules/src/main/scala/fix/OptionSize.scala | 34 +++++++++++++++++++ 5 files changed, 96 insertions(+) create mode 100644 be2-scala/linter/input/src/main/scala/fix/OptionGet.scala create mode 100644 be2-scala/linter/input/src/main/scala/fix/OptionSize.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/OptionGet.scala b/be2-scala/linter/input/src/main/scala/fix/OptionGet.scala new file mode 100644 index 0000000000..33a5c5dfb1 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/OptionGet.scala @@ -0,0 +1,13 @@ +/* +rule = OptionGet + */ +package fix + +object OptionGet { + def test() = { + val o = Option("olivia") + o.get // assert: OptionGet + + Option("layla").get // assert: OptionGet + } +} diff --git a/be2-scala/linter/input/src/main/scala/fix/OptionSize.scala b/be2-scala/linter/input/src/main/scala/fix/OptionSize.scala new file mode 100644 index 0000000000..f8b9130b76 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/OptionSize.scala @@ -0,0 +1,13 @@ +/* +rule = OptionSize + */ +package fix + +object OptionSize { + def test() = { + val o = Option("olivia") + o.size // assert: OptionSize + + Option("layla").size // assert: OptionSize + } +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 37c5025163..75453716dc 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -9,3 +9,5 @@ fix.IncorrectlyNamedExceptions fix.LonelySealedTrait fix.MapGetAndGetOrElse fix.NanComparison +fix.OptionGet +fix.OptionSize diff --git a/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala b/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala new file mode 100644 index 0000000000..6b68e20782 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala @@ -0,0 +1,34 @@ +/* +rule = OptionGet +*/ +package fix + +import scalafix.lint.LintSeverity +import scalafix.v1._ +import scala.meta._ + +case class OptionGetDiag(tree: Tree) extends Diagnostic { + override def message: String = "Use of Option.Get" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Using Option.get defeats the purpose of using Option in the first place. Use the following instead: Option.getOrElse, Option.fold, pattern matching or don't take the value out of the container and map over it to transform it" + + override def position: Position = tree.pos +} + +class OptionGet extends SemanticRule("OptionGet") { + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case t @ Term.Select(Term.Apply.After_4_6_0(Term.Name("Option"), _), Term.Name("get")) => Patch.lint(OptionGetDiag(t)) + case t @ Term.Select(qual, Term.Name("get")) => + val qualType = Util.getType(qual) + if (qualType != null && SymbolMatcher.exact("scala/Option#").matches(qualType)) { + Patch.lint(OptionGetDiag(t)) + } + else Patch.empty + case _ => Patch.empty + }.asPatch + } +} diff --git a/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala b/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala new file mode 100644 index 0000000000..19c15ea57a --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala @@ -0,0 +1,34 @@ +/* +rule = OptionSize +*/ +package fix + +import scalafix.lint.LintSeverity +import scalafix.v1._ +import scala.meta._ + +case class OptionSizeDiag(tree: Tree) extends Diagnostic { + override def message: String = "Prefer Option.isDefined instead of Option.size" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Prefer to use Option.isDefined, Option.isEmpty or Option.nonEmpty instead of Option.size." + + override def position: Position = tree.pos +} + +class OptionSize extends SemanticRule("OptionSize") { + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case t @ Term.Select(Term.Apply.After_4_6_0(Term.Name("Option"), _), Term.Name("size")) => Patch.lint(OptionSizeDiag(t)) + case t @ Term.Select(qual, Term.Name("size")) => + val qualType = Util.getType(qual) + if (qualType != null && SymbolMatcher.exact("scala/Option#").matches(qualType)) { + Patch.lint(OptionSizeDiag(t)) + } + else Patch.empty + case _ => Patch.empty + }.asPatch + } +} From cf66a861eb3221d99cc6cc22e81bb816a8c87050 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 30 May 2024 09:54:19 +0200 Subject: [PATCH 16/23] Added StripMarginOnRegex rule Adapted IllegalFormatString and NanComparison to modification in findDefinitions in Util --- .../main/scala/fix/StripMarginOnRegex.scala | 13 +++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../main/scala/fix/IllegalFormatString.scala | 2 +- .../src/main/scala/fix/NanComparison.scala | 5 +++- .../main/scala/fix/StripMarginOnRegex.scala | 29 +++++++++++++++++++ .../rules/src/main/scala/fix/Util.scala | 2 +- 6 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala b/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala new file mode 100644 index 0000000000..c4718c3e92 --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala @@ -0,0 +1,13 @@ +/* +rule = StripMarginOnRegex + */ +package fix + +object StripMarginOnRegex { + def test() = { +// val regex = "match|this".stripMargin.r // assert: StripMarginOnRegex +// "match|that".stripMargin.r // assert: StripMarginOnRegex + "match_this".stripMargin.r // scalafix: ok; + "match|this".r // scalafix: ok; + } +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 75453716dc..35c117b68e 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -11,3 +11,4 @@ fix.MapGetAndGetOrElse fix.NanComparison fix.OptionGet fix.OptionSize +fix.StripMarginOnRegex diff --git a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala index 4f8cc8a17f..ae985feca3 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/IllegalFormatString.scala @@ -35,7 +35,7 @@ class IllegalFormatString extends SemanticRule("IllegalFormatString") { override def fix(implicit doc: SemanticDocument): Patch = { def getMappedArgs(args: List[Term]): List[Any] = { - findDefinitionsOrdered(doc.tree, args) ++ args.collect { case Lit(value) => value } + (findDefinitionsOrdered(doc.tree, args) ++ args).collect { case Lit(value) => value } } doc.tree.collect { diff --git a/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala index 64ed0bb0f8..da0bf8eee2 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala @@ -34,7 +34,10 @@ class NanComparison extends SemanticRule("NanComparison") { doc.tree.collect { case t @ Term.ApplyInfix.After_4_6_0(lhs, Term.Name("=="), _, Term.ArgClause(List(rhs), _)) => (lhs,rhs) match { - case (Term.Name(_), Term.Name(_)) => matcher(Util.findDefinition(doc.tree, lhs), Util.findDefinition(doc.tree, rhs), t) + case (Term.Name(_), Term.Name(_)) => Util.findDefinitions(doc.tree, Set(lhs, rhs)) match { + case List((_, ld), (_, rd)) => matcher(ld, rd, t) + case _ => Patch.empty + } // Extract definitions and match in the case both are variables case _ => matcher(lhs, rhs, t) } diff --git a/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala new file mode 100644 index 0000000000..40e8682c94 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala @@ -0,0 +1,29 @@ +/* +rule = StripMarginOnRegex +*/ +package fix + +import scalafix.lint.LintSeverity +import scalafix.v1._ +import scala.meta._ + +case class StripMarginOnRegexDiag(tree: Tree) extends Diagnostic { + override def message: String = "Strip margin on regex" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Strip margin will strip | from regex - possible corrupted regex." + + override def position: Position = tree.pos +} + +class StripMarginOnRegex extends SemanticRule("StripMarginOnRegex") { + + override def fix(implicit doc: SemanticDocument): Patch = { + println(doc.tree.structure) + doc.tree.collect { + case t @ Term.Select(Term.Select(str@Lit.String(_), Term.Name("stripMargin")), Term.Name("r")) if str.value.contains('|') => Patch.lint(StripMarginOnRegexDiag(t)) + case _ => Patch.empty + }.asPatch + } +} diff --git a/be2-scala/linter/rules/src/main/scala/fix/Util.scala b/be2-scala/linter/rules/src/main/scala/fix/Util.scala index 9cd8796449..98c4a0ed27 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/Util.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/Util.scala @@ -26,7 +26,7 @@ object Util { def findDefinitions(tree: Tree, nameSet: Set[Term]): List[(Term, Any)] = { tree.collect { - case Defn.Val(_, List(Pat.Var(varName)), _, Lit(value)) if nameSet.exists(_.toString().equals(varName.value)) => nameSet.find(_.toString().equals(varName.value)).get -> value + case Defn.Val(_, List(Pat.Var(varName)), _, value) if nameSet.exists(_.toString().equals(varName.value)) => nameSet.find(_.toString().equals(varName.value)).get -> value case Defn.Var.After_4_7_2(_, List(Pat.Var(varName)), _, value) if nameSet.exists(_.toString().equals(varName.value)) => nameSet.find(_.toString().equals(varName.value)).get -> value } } From 70a9eb5888fa53eb93b6a3f070bc8671b3a7e374 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 30 May 2024 09:54:43 +0200 Subject: [PATCH 17/23] Removed unecessary println --- .../linter/rules/src/main/scala/fix/StripMarginOnRegex.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala index 40e8682c94..a7e93216c4 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala @@ -20,7 +20,6 @@ case class StripMarginOnRegexDiag(tree: Tree) extends Diagnostic { class StripMarginOnRegex extends SemanticRule("StripMarginOnRegex") { override def fix(implicit doc: SemanticDocument): Patch = { - println(doc.tree.structure) doc.tree.collect { case t @ Term.Select(Term.Select(str@Lit.String(_), Term.Name("stripMargin")), Term.Name("r")) if str.value.contains('|') => Patch.lint(StripMarginOnRegexDiag(t)) case _ => Patch.empty From 10add0c3d6cb666325717eee231ee54f3a8351db Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 30 May 2024 09:55:26 +0200 Subject: [PATCH 18/23] Simplified code for StripMarginOnRegex --- .../linter/rules/src/main/scala/fix/StripMarginOnRegex.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala index a7e93216c4..694ddab256 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/StripMarginOnRegex.scala @@ -21,7 +21,7 @@ class StripMarginOnRegex extends SemanticRule("StripMarginOnRegex") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case t @ Term.Select(Term.Select(str@Lit.String(_), Term.Name("stripMargin")), Term.Name("r")) if str.value.contains('|') => Patch.lint(StripMarginOnRegexDiag(t)) + case t @ Term.Select(Term.Select(Lit.String(string), Term.Name("stripMargin")), Term.Name("r")) if string.contains('|') => Patch.lint(StripMarginOnRegexDiag(t)) case _ => Patch.empty }.asPatch } From 36818353976d778c12e5b74165fe12c3bff16b48 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 30 May 2024 11:14:48 +0200 Subject: [PATCH 19/23] Added TryGet rule Improved Util with a matchType function and adapted matching --- .../input/src/main/scala/fix/TryGet.scala | 15 ++++++++++ .../META-INF/services/scalafix.v1.Rule | 1 + .../scala/fix/ComparingFloatingTypes.scala | 4 +-- .../fix/ImpossibleOptionSizeCondition.scala | 3 +- .../main/scala/fix/MapGetAndGetOrElse.scala | 12 ++------ .../rules/src/main/scala/fix/OptionGet.scala | 8 +----- .../rules/src/main/scala/fix/OptionSize.scala | 8 +----- .../rules/src/main/scala/fix/TryGet.scala | 28 +++++++++++++++++++ .../rules/src/main/scala/fix/Util.scala | 7 +++++ 9 files changed, 58 insertions(+), 28 deletions(-) create mode 100644 be2-scala/linter/input/src/main/scala/fix/TryGet.scala create mode 100644 be2-scala/linter/rules/src/main/scala/fix/TryGet.scala diff --git a/be2-scala/linter/input/src/main/scala/fix/TryGet.scala b/be2-scala/linter/input/src/main/scala/fix/TryGet.scala new file mode 100644 index 0000000000..e280c6f68e --- /dev/null +++ b/be2-scala/linter/input/src/main/scala/fix/TryGet.scala @@ -0,0 +1,15 @@ +/* +rule = TryGet + */ +package fix + +import scala.util.Try + +object TryGet { + def test() = { + val o = Try { println("mojave") } + o.get // assert: TryGet + + Try { println("ghost") }.get // assert: TryGet + } +} diff --git a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 35c117b68e..0b8e8f0e46 100644 --- a/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/be2-scala/linter/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -12,3 +12,4 @@ fix.NanComparison fix.OptionGet fix.OptionSize fix.StripMarginOnRegex +fix.TryGet diff --git a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala index 54fe8b6164..5756839fe8 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ComparingFloatingTypes.scala @@ -3,7 +3,6 @@ rule = CatchNpe */ package fix -import fix.Util.getType import scalafix.lint.LintSeverity import scala.meta._ @@ -24,8 +23,7 @@ class ComparingFloatingTypes extends SemanticRule("ComparingFloatingTypes") { override def fix(implicit doc: SemanticDocument): Patch = { def isFloatOrDouble(term: Term): Boolean = { - val floatOrDoubleMatcher = SymbolMatcher.exact("scala/Float#", "scala/Double#") - floatOrDoubleMatcher.matches(getType(term)) + Util.matchType(term, "scala/Float", "scala/Double") } doc.tree.collect { diff --git a/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala b/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala index 0abf668b17..a0d8aa8cc4 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/ImpossibleOptionSizeCondition.scala @@ -3,7 +3,6 @@ rule = ImpossibleOptionSizeCondition */ package fix -import fix.Util.getType import scalafix.lint.LintSeverity import scala.meta._ @@ -26,7 +25,7 @@ class ImpossibleOptionSizeCondition extends SemanticRule("ImpossibleOptionSizeCo doc.tree.collect { case t @ Term.ApplyInfix.After_4_6_0(Term.Select(qual, Term.Name("size")), Term.Name(">") | Term.Name(">="), _, Term.ArgClause(List(comparedValue), _)) - if SymbolMatcher.exact("scala/Option#", "scala/Some#").matches(getType(qual)) => + if Util.matchType(qual, "scala/Option", "scala/Some") => comparedValue match { case Lit.Int(actualValue) if actualValue >= 1 => Patch.lint(ImpossibleOptionSizeConditionDiag(t)) case _ => Patch.empty diff --git a/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala b/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala index c1102fda9c..01bb8a8f8d 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/MapGetAndGetOrElse.scala @@ -22,15 +22,9 @@ class MapGetAndGetOrElse extends SemanticRule("MapGetAndGetOrElse") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case Term.Apply.After_4_6_0(Term.Select( - Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("get")), _), Term.Name("getOrElse")), _) => - qual match { - case variable @ Term.Name(_) - if SymbolMatcher.normalized("scala/collection/immutable/Map#", "scala/collection/mutable/Map#") - .matches(Util.getType(variable)) => Patch.lint(MapGetAndGetOrElseDiag(variable)) - case Term.Apply.After_4_6_0(Term.Name("Map"), _) => Patch.lint(MapGetAndGetOrElseDiag(qual)) - case _ => Patch.empty - } + case Term.Apply.After_4_6_0(Term.Select(Term.Apply.After_4_6_0(Term.Select(qual, Term.Name("get")), _), Term.Name("getOrElse")), _) + if Util.matchType(qual, "scala/collection/immutable/Map", "scala/collection/mutable/Map", "scala/Predef.Map") + => Patch.lint(MapGetAndGetOrElseDiag(qual)) case _ => Patch.empty }.asPatch } diff --git a/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala b/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala index 6b68e20782..d53d889e91 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/OptionGet.scala @@ -21,13 +21,7 @@ class OptionGet extends SemanticRule("OptionGet") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case t @ Term.Select(Term.Apply.After_4_6_0(Term.Name("Option"), _), Term.Name("get")) => Patch.lint(OptionGetDiag(t)) - case t @ Term.Select(qual, Term.Name("get")) => - val qualType = Util.getType(qual) - if (qualType != null && SymbolMatcher.exact("scala/Option#").matches(qualType)) { - Patch.lint(OptionGetDiag(t)) - } - else Patch.empty + case t @ Term.Select(qual, Term.Name("get")) if Util.matchType(qual, "scala/Option") => Patch.lint(OptionGetDiag(t)) case _ => Patch.empty }.asPatch } diff --git a/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala b/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala index 19c15ea57a..6715995b06 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/OptionSize.scala @@ -21,13 +21,7 @@ class OptionSize extends SemanticRule("OptionSize") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case t @ Term.Select(Term.Apply.After_4_6_0(Term.Name("Option"), _), Term.Name("size")) => Patch.lint(OptionSizeDiag(t)) - case t @ Term.Select(qual, Term.Name("size")) => - val qualType = Util.getType(qual) - if (qualType != null && SymbolMatcher.exact("scala/Option#").matches(qualType)) { - Patch.lint(OptionSizeDiag(t)) - } - else Patch.empty + case t @ Term.Select(qual, Term.Name("size")) if Util.matchType(qual, "scala/Option") => Patch.lint(OptionSizeDiag(t)) case _ => Patch.empty }.asPatch } diff --git a/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala b/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala new file mode 100644 index 0000000000..c161f05780 --- /dev/null +++ b/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala @@ -0,0 +1,28 @@ +/* +rule = TryGet +*/ +package fix + +import scalafix.lint.LintSeverity +import scalafix.v1._ +import scala.meta._ + +case class TryGetDiag(tree: Tree) extends Diagnostic { + override def message: String = "Use of Try.Get" + + override def severity: LintSeverity = LintSeverity.Error + + override def explanation: String = "Using Try.get is unsafe because it will throw the underlying exception in case of a Failure. Use the following instead: Try.getOrElse, Try.fold, pattern matching or don't take the value out of the container and map over it to transform it." + + override def position: Position = tree.pos +} + +class TryGet extends SemanticRule("TryGet") { + + override def fix(implicit doc: SemanticDocument): Patch = { + doc.tree.collect { + case t @ Term.Select(qual, Term.Name("get")) if Util.matchType(qual, "scala/util/Try") => Patch.lint(OptionGetDiag(t)) + case _ => Patch.empty + }.asPatch + } +} diff --git a/be2-scala/linter/rules/src/main/scala/fix/Util.scala b/be2-scala/linter/rules/src/main/scala/fix/Util.scala index 98c4a0ed27..193c0fd719 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/Util.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/Util.scala @@ -15,6 +15,13 @@ object Util { } } + def matchType(term: Term, symbols: String*)(implicit doc: SemanticDocument): Boolean = { + val symbolMatcher = SymbolMatcher.normalized(symbols: _*) + symbolMatcher.matches(term.symbol) || Option(getType(term)).exists(symbolMatcher.matches) + // Checks the term symbol matches that of the symbol (i.e. when we use the type directly), + // or if the type of the variable matches. We use an option as getType is not null safe. + } + def findDefinition(tree: Tree, name: Term): Any = { tree.collect { case Defn.Val(_, List(Pat.Var(varName)), _, value) From 39dfad3f3873ad64d20cd9dbb9c9dee8198b7bf2 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Thu, 30 May 2024 11:21:12 +0200 Subject: [PATCH 20/23] Fixed wrong diag --- be2-scala/linter/rules/src/main/scala/fix/TryGet.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala b/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala index c161f05780..8e07f1fa2f 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/TryGet.scala @@ -21,7 +21,7 @@ class TryGet extends SemanticRule("TryGet") { override def fix(implicit doc: SemanticDocument): Patch = { doc.tree.collect { - case t @ Term.Select(qual, Term.Name("get")) if Util.matchType(qual, "scala/util/Try") => Patch.lint(OptionGetDiag(t)) + case t @ Term.Select(qual, Term.Name("get")) if Util.matchType(qual, "scala/util/Try") => Patch.lint(TryGetDiag(t)) case _ => Patch.empty }.asPatch } From a83677192205b3ba2ec5b5f67ff402fd1ec0cf9e Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Fri, 31 May 2024 09:04:45 +0200 Subject: [PATCH 21/23] Made getType null safe --- be2-scala/linter/rules/src/main/scala/fix/Util.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/be2-scala/linter/rules/src/main/scala/fix/Util.scala b/be2-scala/linter/rules/src/main/scala/fix/Util.scala index 193c0fd719..bd55e2f27c 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/Util.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/Util.scala @@ -9,17 +9,17 @@ object Util { term.symbol.info match { case Some(symInfo) => symInfo.signature match { case ValueSignature(TypeRef(_, symbol, _)) => symbol - case _ => null + case _ => Symbol.None } - case _ => null + case _ => Symbol.None } } def matchType(term: Term, symbols: String*)(implicit doc: SemanticDocument): Boolean = { val symbolMatcher = SymbolMatcher.normalized(symbols: _*) - symbolMatcher.matches(term.symbol) || Option(getType(term)).exists(symbolMatcher.matches) + symbolMatcher.matches(term.symbol) || symbolMatcher.matches(getType(term)) // Checks the term symbol matches that of the symbol (i.e. when we use the type directly), - // or if the type of the variable matches. We use an option as getType is not null safe. + // or if the type of the variable matches. } def findDefinition(tree: Tree, name: Term): Any = { From 4eb6797512346a4af891554e6ace20cf46a35a30 Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Tue, 4 Jun 2024 10:46:31 +0200 Subject: [PATCH 22/23] Uncommented code --- .../linter/input/src/main/scala/fix/StripMarginOnRegex.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala b/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala index c4718c3e92..d60007cf5f 100644 --- a/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala +++ b/be2-scala/linter/input/src/main/scala/fix/StripMarginOnRegex.scala @@ -5,8 +5,8 @@ package fix object StripMarginOnRegex { def test() = { -// val regex = "match|this".stripMargin.r // assert: StripMarginOnRegex -// "match|that".stripMargin.r // assert: StripMarginOnRegex + val regex = "match|this".stripMargin.r // assert: StripMarginOnRegex + "match|that".stripMargin.r // assert: StripMarginOnRegex "match_this".stripMargin.r // scalafix: ok; "match|this".r // scalafix: ok; } From 9c3227482b3f211e5b2e39ca513b64ee8c5b041b Mon Sep 17 00:00:00 2001 From: Thibault Czarniak Date: Wed, 5 Jun 2024 10:10:57 +0200 Subject: [PATCH 23/23] Added .equals() support in NanComparison --- .../src/main/scala/fix/NanComparison.scala | 4 ++++ .../src/main/scala/fix/NanComparison.scala | 22 +++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala b/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala index eed0bf7c72..6189182dd2 100644 --- a/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala +++ b/be2-scala/linter/input/src/main/scala/fix/NanComparison.scala @@ -9,6 +9,8 @@ object NanComparison { val d = 0.5d d == Double.NaN // assert: NanComparison Double.NaN == d // assert: NanComparison + d.equals(Double.NaN) // assert: NanComparison + Double.NaN.equals(d) // assert: NanComparison val f = 0.5f f == Double.NaN // assert: NanComparison @@ -20,6 +22,8 @@ object NanComparison { d == g // assert: NanComparison g == d // assert: NanComparison + d.equals(g) // assert: NanComparison + g.equals(d) // assert: NanComparison } } diff --git a/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala index da0bf8eee2..36b83a72a3 100644 --- a/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala +++ b/be2-scala/linter/rules/src/main/scala/fix/NanComparison.scala @@ -31,16 +31,20 @@ class NanComparison extends SemanticRule("NanComparison") { } } - doc.tree.collect { - case t @ Term.ApplyInfix.After_4_6_0(lhs, Term.Name("=="), _, Term.ArgClause(List(rhs), _)) => - (lhs,rhs) match { - case (Term.Name(_), Term.Name(_)) => Util.findDefinitions(doc.tree, Set(lhs, rhs)) match { - case List((_, ld), (_, rd)) => matcher(ld, rd, t) - case _ => Patch.empty - } - // Extract definitions and match in the case both are variables - case _ => matcher(lhs, rhs, t) + def rule(lhs: Term, rhs: Term, t: Term): Patch = { + (lhs,rhs) match { + case (Term.Name(_), Term.Name(_)) => Util.findDefinitions(doc.tree, Set(lhs, rhs)) match { + case List((_, ld), (_, rd)) => matcher(ld, rd, t) + case _ => Patch.empty } + // Extract definitions and match in the case both are variables + case _ => matcher(lhs, rhs, t) + } + } + + doc.tree.collect { + case t @ Term.ApplyInfix.After_4_6_0(lhs, Term.Name("==") | Term.Name("!="), _, Term.ArgClause(List(rhs), _)) => rule(lhs, rhs, t) + case t @ Term.Apply.After_4_6_0(Term.Select(lhs, Term.Name("equals")), Term.ArgClause(List(right), _)) => rule(lhs, right, t) }.asPatch } }