From 82ee547e9e1e78710e1e6bad7a3c6e8aaecdec0d Mon Sep 17 00:00:00 2001 From: cobr123 Date: Sun, 18 Sep 2022 13:12:35 +0200 Subject: [PATCH 01/19] SCL-9153 Warn about calling .toString() on Option --- .../META-INF/scala-plugin-common.xml | 6 ++ .../OptionToString.html | 17 ++++ .../messages/ScalaInspectionBundle.properties | 4 + .../OptionToStringInspection.scala | 32 +++++++ .../OptionToStringInspectionTest.scala | 89 +++++++++++++++++++ 5 files changed, 148 insertions(+) create mode 100644 scala/scala-impl/resources/inspectionDescriptions/OptionToString.html create mode 100644 scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala create mode 100644 scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala diff --git a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml index afc801d2f0b..376280994ec 100644 --- a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml +++ b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml @@ -1769,6 +1769,12 @@ groupPathKey="grouppath.scala.collections" groupKey="group.options" shortName="IfElseToFilterdOption" level="WARNING" enabledByDefault="true" language="Scala"/> + + +

Reports and replaces toString with getOrElse(throw new NoSuchElementException()).

+ +

getOrElse will prevent from getting string "Some(a)" instead of expected string "a".

+ +

Example:

+

+  Option(a).toString
+
+

After the quick-fix is applied:

+

+  Option(a).getOrElse(throw new NoSuchElementException())
+
+ + + \ No newline at end of file diff --git a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties index e0030f3e873..b5372ad5ad1 100644 --- a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties +++ b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties @@ -133,6 +133,7 @@ displayname.map.and.getorelse.false.to.exists=Map and getOrElse(false) to exists displayname.getorelse.null.to.ornull=GetOrElse(null) to orNull displayname.emulated.option.x=Emulated Option(x) displayname.change.to.filter=Change to filter +displayname.change.tostring.to.getorelse.on.option=Change toString to getOrElse displayname.some.to.option=Some to Option displayname.filter.after.sort=Filter after sort displayname.redundant.collection.conversion=Redundant collection conversion @@ -307,6 +308,9 @@ fold.true.and.hint=Replace fold with forall ### org/jetbrains/plugins/scala/codeInspection/collections/GetGetOrElseInspection.scala get.getOrElse.hint=Replace with getOrElse(key, defaultValue) +### org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala +option.getOrElse.hint=Replace with getOrElse(defaultValue) + ### org/jetbrains/plugins/scala/codeInspection/collections/GetOrElseNullInspection.scala getOrElse.null.hint=Replace getOrElse(null) with orNull diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala new file mode 100644 index 00000000000..32cc7410150 --- /dev/null +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala @@ -0,0 +1,32 @@ +package org.jetbrains.plugins.scala.codeInspection.collections + +import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle +import org.jetbrains.plugins.scala.lang.psi.api.expr.ScExpression +import org.jetbrains.plugins.scala.lang.psi.types.ScParameterizedType +import org.jetbrains.plugins.scala.lang.psi.types.api.designator.ScDesignatorType + +import scala.collection.immutable.ArraySeq + +class OptionToStringInspection extends OperationOnCollectionInspection { + override def possibleSimplificationTypes: ArraySeq[SimplificationType] = ArraySeq(OptionToStringInspection) +} + +object OptionToStringInspection extends SimplificationType { + override def hint: String = ScalaInspectionBundle.message("option.getOrElse.hint") + + override def getSimplification(expr: ScExpression): Option[Simplification] = expr match { + case `.toString`(scalaNone()) => None + case `.toString`(opt) if isOption(opt) => + opt.getTypeAfterImplicitConversion().tr match { + case Right(t: ScParameterizedType) if t.typeArguments.forall(SizeToLength.isString) => + Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + case Right(t: ScDesignatorType) if (t.extractDesignatorSingleton match { + case Some(t: ScParameterizedType) if t.typeArguments.forall(SizeToLength.isString) => true + case _ => false + }) => Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + case _ => + Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + } + case _ => None + } +} \ No newline at end of file diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala new file mode 100644 index 00000000000..5889c030407 --- /dev/null +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -0,0 +1,89 @@ +package org.jetbrains.plugins.scala.codeInspection.collections + +import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle +import org.junit.Assert.assertEquals + +class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest { + + override protected val classOfInspection: Class[_ <: OperationOnCollectionInspection] = + classOf[OptionToStringInspection] + + override protected val hint: String = ScalaInspectionBundle.message("option.getOrElse.hint") + + def testNone(): Unit = { + try { + doTest( + s"""${START}None.toString$END""", + """None.toString""", + """None.toString""" + ) + } catch { + case e: AssertionError => assertEquals(e.getMessage, "Highlights not found: Replace with getOrElse(defaultValue)") + } + } + + def testOptionVal(): Unit = { + doTest( + s"""val i = Option("hello"); ${START}i.toString$END""", + """val i = Option("hello"); i.toString""", + """val i = Option("hello"); i.getOrElse(throw new NoSuchElementException())""" + ) + } + + def testFunctionExpression(): Unit = { + doTest( + s"""${START}sys.env.get("VARIABLE").toString$END""", + """sys.env.get("VARIABLE").toString""", + """sys.env.get("VARIABLE").getOrElse(throw new NoSuchElementException())""" + ) + } + + def testSomeConstant(): Unit = { + doTest( + s"""${START}Some("constant").toString$END""", + """Some("constant").toString""", + """Some("constant").getOrElse(throw new NoSuchElementException())""" + ) + } + + def testOptionConstant(): Unit = { + doTest( + s"""${START}Option("constant").toString$END""", + """Option("constant").toString""", + """Option("constant").getOrElse(throw new NoSuchElementException())""" + ) + } + + def testOptionValNotString(): Unit = { + doTest( + s"""val i = Option(1); ${START}i.toString$END""", + """val i = Option(1); i.toString""", + """val i = Option(1); i.map(_.toString).getOrElse(throw new NoSuchElementException())""" + ) + } + + def testFunctionExpressionNotString(): Unit = { + doTest( + s"""def getSomeOne():Option[Int] = Some(1); ${START}getSomeOne().toString$END""", + """def getSomeOne():Option[Int] = Some(1); getSomeOne().toString""", + """def getSomeOne():Option[Int] = Some(1); getSomeOne().map(_.toString).getOrElse(throw new NoSuchElementException())""" + ) + } + + def testSomeConstantNotString(): Unit = { + doTest( + s"""${START}Some(1).toString$END""", + """Some(1).toString""", + """Some(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" + ) + } + + def testOptionConstantNotString(): Unit = { + doTest( + s"""${START}Option(1).toString$END""", + """Option(1).toString""", + """Option(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" + ) + } + +} From 0850ae1141ace75e8a6cfa76c169230d39da2729 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Mon, 19 Sep 2022 14:27:13 +0200 Subject: [PATCH 02/19] add case for ScProjectionType --- .../OptionToStringInspection.scala | 36 ++++++++++++++----- .../OptionToStringInspectionTest.scala | 8 +++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala index 32cc7410150..7013389a2de 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala @@ -3,7 +3,7 @@ package org.jetbrains.plugins.scala.codeInspection.collections import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle import org.jetbrains.plugins.scala.lang.psi.api.expr.ScExpression import org.jetbrains.plugins.scala.lang.psi.types.ScParameterizedType -import org.jetbrains.plugins.scala.lang.psi.types.api.designator.ScDesignatorType +import org.jetbrains.plugins.scala.lang.psi.types.api.designator.{ScDesignatorType, ScProjectionType} import scala.collection.immutable.ArraySeq @@ -18,15 +18,33 @@ object OptionToStringInspection extends SimplificationType { case `.toString`(scalaNone()) => None case `.toString`(opt) if isOption(opt) => opt.getTypeAfterImplicitConversion().tr match { - case Right(t: ScParameterizedType) if t.typeArguments.forall(SizeToLength.isString) => - Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) - case Right(t: ScDesignatorType) if (t.extractDesignatorSingleton match { - case Some(t: ScParameterizedType) if t.typeArguments.forall(SizeToLength.isString) => true - case _ => false - }) => Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) - case _ => - Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + case Right(t: ScParameterizedType) if isStringInOption(t) => onString(opt, expr) + case Right(t: ScDesignatorType) if isStringInOption(t) => onString(opt, expr) + case Right(t: ScProjectionType) if isStringInOption(t) => onString(opt, expr) + case _ => onNotString(opt, expr) } case _ => None } + + private def isStringInOption(t: ScParameterizedType): Boolean = t.typeArguments.forall(SizeToLength.isString) + + private def isStringInOption(t: ScDesignatorType): Boolean = { + t.extractDesignatorSingleton match { + case Some(t: ScParameterizedType) if isStringInOption(t) => true + case _ => false + } + } + + private def isStringInOption(t: ScProjectionType): Boolean = { + t.extractDesignatorSingleton match { + case Some(t: ScParameterizedType) if isStringInOption(t) => true + case _ => false + } + } + + private def onString(opt: ScExpression, expr: ScExpression): Option[Simplification] = + Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + + private def onNotString(opt: ScExpression, expr: ScExpression): Option[Simplification] = + Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) } \ No newline at end of file diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 5889c030407..668eb40cb4a 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -30,6 +30,14 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest ) } + def testOptionValInObject(): Unit = { + doTest( + s"""object Test { val i = Option("hello"); ${START}i.toString$END }""", + """object Test { val i = Option("hello"); i.toString }""", + """object Test { val i = Option("hello"); i.getOrElse(throw new NoSuchElementException()) }""" + ) + } + def testFunctionExpression(): Unit = { doTest( s"""${START}sys.env.get("VARIABLE").toString$END""", From a200fbc3be19b734468bae70584f5e732670e091 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Tue, 20 Sep 2022 09:28:58 +0200 Subject: [PATCH 03/19] fix highlight --- .../OptionToStringInspection.scala | 4 ++-- .../OptionToStringInspectionTest.scala | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala index 7013389a2de..7ba0cfaf6c3 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala @@ -43,8 +43,8 @@ object OptionToStringInspection extends SimplificationType { } private def onString(opt: ScExpression, expr: ScExpression): Option[Simplification] = - Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(opt)) private def onNotString(opt: ScExpression, expr: ScExpression): Option[Simplification] = - Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(expr)) + Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(opt)) } \ No newline at end of file diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 668eb40cb4a..69a01e0f8d0 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -13,7 +13,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testNone(): Unit = { try { doTest( - s"""${START}None.toString$END""", + s"""None.${START}toString$END""", """None.toString""", """None.toString""" ) @@ -24,7 +24,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testOptionVal(): Unit = { doTest( - s"""val i = Option("hello"); ${START}i.toString$END""", + s"""val i = Option("hello"); i.${START}toString$END""", """val i = Option("hello"); i.toString""", """val i = Option("hello"); i.getOrElse(throw new NoSuchElementException())""" ) @@ -32,7 +32,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testOptionValInObject(): Unit = { doTest( - s"""object Test { val i = Option("hello"); ${START}i.toString$END }""", + s"""object Test { val i = Option("hello"); i.${START}toString$END }""", """object Test { val i = Option("hello"); i.toString }""", """object Test { val i = Option("hello"); i.getOrElse(throw new NoSuchElementException()) }""" ) @@ -40,7 +40,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testFunctionExpression(): Unit = { doTest( - s"""${START}sys.env.get("VARIABLE").toString$END""", + s"""sys.env.get("VARIABLE").${START}toString$END""", """sys.env.get("VARIABLE").toString""", """sys.env.get("VARIABLE").getOrElse(throw new NoSuchElementException())""" ) @@ -48,7 +48,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testSomeConstant(): Unit = { doTest( - s"""${START}Some("constant").toString$END""", + s"""Some("constant").${START}toString$END""", """Some("constant").toString""", """Some("constant").getOrElse(throw new NoSuchElementException())""" ) @@ -56,7 +56,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testOptionConstant(): Unit = { doTest( - s"""${START}Option("constant").toString$END""", + s"""Option("constant").${START}toString$END""", """Option("constant").toString""", """Option("constant").getOrElse(throw new NoSuchElementException())""" ) @@ -64,7 +64,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testOptionValNotString(): Unit = { doTest( - s"""val i = Option(1); ${START}i.toString$END""", + s"""val i = Option(1); i.${START}toString$END""", """val i = Option(1); i.toString""", """val i = Option(1); i.map(_.toString).getOrElse(throw new NoSuchElementException())""" ) @@ -72,7 +72,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testFunctionExpressionNotString(): Unit = { doTest( - s"""def getSomeOne():Option[Int] = Some(1); ${START}getSomeOne().toString$END""", + s"""def getSomeOne():Option[Int] = Some(1); getSomeOne().${START}toString$END""", """def getSomeOne():Option[Int] = Some(1); getSomeOne().toString""", """def getSomeOne():Option[Int] = Some(1); getSomeOne().map(_.toString).getOrElse(throw new NoSuchElementException())""" ) @@ -80,7 +80,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testSomeConstantNotString(): Unit = { doTest( - s"""${START}Some(1).toString$END""", + s"""Some(1).${START}toString$END""", """Some(1).toString""", """Some(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" ) @@ -88,7 +88,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest def testOptionConstantNotString(): Unit = { doTest( - s"""${START}Option(1).toString$END""", + s"""Option(1).${START}toString$END""", """Option(1).toString""", """Option(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" ) From cfa27731e50220cd41cc74d905a096c1b1815895 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Tue, 20 Sep 2022 09:31:43 +0200 Subject: [PATCH 04/19] set enabledByDefault to false --- scala/scala-impl/resources/META-INF/scala-plugin-common.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml index 376280994ec..416a3f5da07 100644 --- a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml +++ b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml @@ -1774,7 +1774,7 @@ key="displayname.change.tostring.to.getorelse.on.option" groupPathKey="grouppath.scala.collections" groupKey="group.options" shortName="OptionToString" level="WARNING" - enabledByDefault="true" language="Scala"/> + enabledByDefault="false" language="Scala"/> Date: Wed, 21 Sep 2022 10:23:52 +0200 Subject: [PATCH 05/19] extract method getToStringToMkStringSimplification --- .../OptionToString.html | 6 +- .../messages/ScalaInspectionBundle.properties | 2 +- .../MakeArrayToStringInspection.scala | 47 +------ .../OptionToStringInspection.scala | 38 +----- .../codeInspection/collections/package.scala | 44 +++++- .../OptionToStringInspectionTest.scala | 125 +++++++++++++++--- 6 files changed, 159 insertions(+), 103 deletions(-) diff --git a/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html b/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html index a3bafca61f4..9373481417f 100644 --- a/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html +++ b/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html @@ -1,8 +1,8 @@ -

Reports and replaces toString with getOrElse(throw new NoSuchElementException()).

+

Reports and replaces toString with mkString.

-

getOrElse will prevent from getting string "Some(a)" instead of expected string "a".

+

mkString will prevent from getting string "Some(a)" instead of expected string "a".

Example:


@@ -10,7 +10,7 @@

After the quick-fix is applied:


-  Option(a).getOrElse(throw new NoSuchElementException())
+  Option(a).mkString
diff --git a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties index b5372ad5ad1..aea9a349237 100644 --- a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties +++ b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties @@ -309,7 +309,7 @@ fold.true.and.hint=Replace fold with forall get.getOrElse.hint=Replace with getOrElse(key, defaultValue) ### org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala -option.getOrElse.hint=Replace with getOrElse(defaultValue) +option.mkString.hint=Replace with mkString ### org/jetbrains/plugins/scala/codeInspection/collections/GetOrElseNullInspection.scala getOrElse.null.hint=Replace getOrElse(null) with orNull diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala index cb456872bea..7527756e773 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala @@ -2,60 +2,19 @@ package org.jetbrains.plugins.scala package codeInspection package collections -import org.jetbrains.plugins.scala.lang.psi.api.base.ScInterpolatedStringLiteral import org.jetbrains.plugins.scala.lang.psi.api.expr.ScExpression import scala.collection.immutable.ArraySeq -class MakeArrayToStringInspection extends OperationOnCollectionInspection { +class MakeArrayToStringInspection extends OperationOnCollectionInspection { override def possibleSimplificationTypes: ArraySeq[SimplificationType] = ArraySeq(MakeArrayToStringInspection) } object MakeArrayToStringInspection extends SimplificationType { override def hint: String = ScalaInspectionBundle.message("format.with.mkstring") - private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) - private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) - private val `+` = invocation(Set("+")) - private val mkString = """mkString("Array(", ", ", ")")""" - override def getSimplification(expr: ScExpression): Option[Simplification] = { - expr match { - // TODO infix notation? - case `.toString`(array) if isArray(array) => - // array.toString - Some(replace(expr).withText(invocationText(array, mkString)).highlightFrom(array)) - case someString `+` array if isString(someString) && isArray(array) => - // "string" + array - Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) - case array `+` someString if isString(someString) && isArray(array) => - // array + "string" - Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) - case _ if isArray(expr) => - def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, mkString)).highlightFrom(expr) - - expr.getParent match { - case _: ScInterpolatedStringLiteral => - // s"start $array end" - Some(result.wrapInBlock()) - case null => None - case parent => - parent.getParent match { - case `.print`(_, args@_*) if args.contains(expr) => - // System.out.println(array) - Some(result) - case `print` (args@_*) if args.contains(expr) => - // println(array) - Some(result) - case _: ScInterpolatedStringLiteral => - // s"start ${array} end" - Some(result) - case _ => None - } - } - case _ => - None - } - } + override def getSimplification(expr: ScExpression): Option[Simplification] = + getToStringToMkStringSimplification(expr, isArray, mkString, replace) } diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala index 7ba0cfaf6c3..fb22fd1ce4d 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala @@ -2,8 +2,6 @@ package org.jetbrains.plugins.scala.codeInspection.collections import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle import org.jetbrains.plugins.scala.lang.psi.api.expr.ScExpression -import org.jetbrains.plugins.scala.lang.psi.types.ScParameterizedType -import org.jetbrains.plugins.scala.lang.psi.types.api.designator.{ScDesignatorType, ScProjectionType} import scala.collection.immutable.ArraySeq @@ -12,39 +10,11 @@ class OptionToStringInspection extends OperationOnCollectionInspection { } object OptionToStringInspection extends SimplificationType { - override def hint: String = ScalaInspectionBundle.message("option.getOrElse.hint") - override def getSimplification(expr: ScExpression): Option[Simplification] = expr match { - case `.toString`(scalaNone()) => None - case `.toString`(opt) if isOption(opt) => - opt.getTypeAfterImplicitConversion().tr match { - case Right(t: ScParameterizedType) if isStringInOption(t) => onString(opt, expr) - case Right(t: ScDesignatorType) if isStringInOption(t) => onString(opt, expr) - case Right(t: ScProjectionType) if isStringInOption(t) => onString(opt, expr) - case _ => onNotString(opt, expr) - } - case _ => None - } + override def hint: String = ScalaInspectionBundle.message("option.mkString.hint") - private def isStringInOption(t: ScParameterizedType): Boolean = t.typeArguments.forall(SizeToLength.isString) + private val mkString = """mkString""" - private def isStringInOption(t: ScDesignatorType): Boolean = { - t.extractDesignatorSingleton match { - case Some(t: ScParameterizedType) if isStringInOption(t) => true - case _ => false - } - } - - private def isStringInOption(t: ScProjectionType): Boolean = { - t.extractDesignatorSingleton match { - case Some(t: ScParameterizedType) if isStringInOption(t) => true - case _ => false - } - } - - private def onString(opt: ScExpression, expr: ScExpression): Option[Simplification] = - Some(replace(expr).withText(invocationText(opt, "getOrElse(throw new NoSuchElementException())")).highlightFrom(opt)) - - private def onNotString(opt: ScExpression, expr: ScExpression): Option[Simplification] = - Some(replace(expr).withText(invocationText(opt, "map(_.toString).getOrElse(throw new NoSuchElementException())")).highlightFrom(opt)) + override def getSimplification(expr: ScExpression): Option[Simplification] = + getToStringToMkStringSimplification(expr, isOption, mkString, replace) } \ No newline at end of file diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 0fe2d1ddec3..2ef38c583ed 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -9,7 +9,7 @@ import org.jetbrains.plugins.scala.extensions._ import org.jetbrains.plugins.scala.lang.psi.ScalaPsiUtil import org.jetbrains.plugins.scala.lang.psi.api.ScalaRecursiveElementVisitor import org.jetbrains.plugins.scala.lang.psi.api.base.patterns.ScCaseClauses -import org.jetbrains.plugins.scala.lang.psi.api.base.{ScLiteral, ScReference} +import org.jetbrains.plugins.scala.lang.psi.api.base.{ScInterpolatedStringLiteral, ScLiteral, ScReference} import org.jetbrains.plugins.scala.lang.psi.api.expr._ import org.jetbrains.plugins.scala.lang.psi.api.statements.params.ScParameter import org.jetbrains.plugins.scala.lang.psi.api.statements.{ScFunction, ScFunctionDefinition, ScValue, ScVariable} @@ -512,5 +512,47 @@ package object collections { def start: Int = elem.getTextRange.getStartOffset def end: Int = elem.getTextRange.getEndOffset } + + private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) + private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) + + private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isFound: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { + expr match { + // TODO infix notation? + case `.toString`(array) if isFound(array) => + // array.toString + Some(replace(expr).withText(invocationText(array, mkString)).highlightFrom(array)) + case someString `+` array if isString(someString) && isFound(array) => + // "string" + array + Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) + case array `+` someString if isString(someString) && isFound(array) => + // array + "string" + Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) + case _ if isFound(expr) => + def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, mkString)).highlightFrom(expr) + + expr.getParent match { + case _: ScInterpolatedStringLiteral => + // s"start $array end" + Some(result.wrapInBlock()) + case null => None + case parent => + parent.getParent match { + case `.print`(_, args@_*) if args.contains(expr) => + // System.out.println(array) + Some(result) + case `print`(args@_*) if args.contains(expr) => + // println(array) + Some(result) + case _: ScInterpolatedStringLiteral => + // s"start ${array} end" + Some(result) + case _ => None + } + } + case _ => + None + } + } } diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 69a01e0f8d0..3fa12d7e9fb 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -1,32 +1,27 @@ package org.jetbrains.plugins.scala.codeInspection.collections import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle -import org.junit.Assert.assertEquals class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest { override protected val classOfInspection: Class[_ <: OperationOnCollectionInspection] = classOf[OptionToStringInspection] - override protected val hint: String = ScalaInspectionBundle.message("option.getOrElse.hint") + override protected val hint: String = ScalaInspectionBundle.message("option.mkString.hint") def testNone(): Unit = { - try { - doTest( - s"""None.${START}toString$END""", - """None.toString""", - """None.toString""" - ) - } catch { - case e: AssertionError => assertEquals(e.getMessage, "Highlights not found: Replace with getOrElse(defaultValue)") - } + doTest( + s"""None.${START}toString$END""", + """None.toString""", + """None.mkString""" + ) } def testOptionVal(): Unit = { doTest( s"""val i = Option("hello"); i.${START}toString$END""", """val i = Option("hello"); i.toString""", - """val i = Option("hello"); i.getOrElse(throw new NoSuchElementException())""" + """val i = Option("hello"); i.mkString""" ) } @@ -34,7 +29,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""object Test { val i = Option("hello"); i.${START}toString$END }""", """object Test { val i = Option("hello"); i.toString }""", - """object Test { val i = Option("hello"); i.getOrElse(throw new NoSuchElementException()) }""" + """object Test { val i = Option("hello"); i.mkString }""" ) } @@ -42,7 +37,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""sys.env.get("VARIABLE").${START}toString$END""", """sys.env.get("VARIABLE").toString""", - """sys.env.get("VARIABLE").getOrElse(throw new NoSuchElementException())""" + """sys.env.get("VARIABLE").mkString""" ) } @@ -50,7 +45,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Some("constant").${START}toString$END""", """Some("constant").toString""", - """Some("constant").getOrElse(throw new NoSuchElementException())""" + """Some("constant").mkString""" ) } @@ -58,7 +53,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Option("constant").${START}toString$END""", """Option("constant").toString""", - """Option("constant").getOrElse(throw new NoSuchElementException())""" + """Option("constant").mkString""" ) } @@ -66,7 +61,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""val i = Option(1); i.${START}toString$END""", """val i = Option(1); i.toString""", - """val i = Option(1); i.map(_.toString).getOrElse(throw new NoSuchElementException())""" + """val i = Option(1); i.mkString""" ) } @@ -74,7 +69,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""def getSomeOne():Option[Int] = Some(1); getSomeOne().${START}toString$END""", """def getSomeOne():Option[Int] = Some(1); getSomeOne().toString""", - """def getSomeOne():Option[Int] = Some(1); getSomeOne().map(_.toString).getOrElse(throw new NoSuchElementException())""" + """def getSomeOne():Option[Int] = Some(1); getSomeOne().mkString""" ) } @@ -82,7 +77,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Some(1).${START}toString$END""", """Some(1).toString""", - """Some(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" + """Some(1).mkString""" ) } @@ -90,8 +85,98 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Option(1).${START}toString$END""", """Option(1).toString""", - """Option(1).map(_.toString).getOrElse(throw new NoSuchElementException())""" + """Option(1).mkString""" ) } + def testPrint(): Unit = { + doTest( + s""" + |println(${START}Option(3)$END) + """.stripMargin, + """ + |println(Option(3)) + """.stripMargin, + """ + |println(Option(3).mkString) + """.stripMargin) + } + + def testSystemPrint(): Unit = { + doTest( + s""" + |System.out.println(${START}Option(3)$END) + """.stripMargin, + """ + |System.out.println(Option(3)) + """.stripMargin, + """ + |System.out.println(Option(3).mkString) + """.stripMargin) + } + + def testInterpolatedString(): Unit = { + doTest( + s""" + |val a = Option(3) + |s"before $$${START}a$END after" + """.stripMargin, + """ + |val a = Option(3) + |s"before $a after" + """.stripMargin, + """ + |val a = Option(3) + |s"before ${a.mkString} after" + """.stripMargin) + } + + def testBlockInInterpolatedString(): Unit = { + doTest( + s""" + |val a = Option(3) + |s"before $${${START}a$END} after" + """.stripMargin, + """ + |val a = Option(3) + |s"before ${a} after" + """.stripMargin, + """ + |val a = Option(3) + |s"before ${a.mkString} after" + """.stripMargin) + } + + def testAny2String(): Unit = { + doTest( + s""" + |val a = Option(3) + |${START}a$END + "test" + """.stripMargin, + """ + |val a = Option(3) + |a + "test" + """.stripMargin, + """ + |val a = Option(3) + |a.mkString + "test" + """.stripMargin) + } + + def testAddToString(): Unit = { + doTest( + s""" + |val a = Option(3) + |"test" + ${START}a$END + """.stripMargin, + """ + |val a = Option(3) + |"test" + a + """.stripMargin, + """ + |val a = Option(3) + |"test" + a.mkString + """.stripMargin) + } + } From 114f116d684c0ccbe87096845443002ece01a904 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 10:32:03 +0200 Subject: [PATCH 06/19] rename inspection --- scala/scala-impl/resources/META-INF/scala-plugin-common.xml | 2 +- .../resources/messages/ScalaInspectionBundle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml index 416a3f5da07..35eca3d77e0 100644 --- a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml +++ b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml @@ -1771,7 +1771,7 @@ enabledByDefault="true" language="Scala"/> diff --git a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties index aea9a349237..ad0a879a253 100644 --- a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties +++ b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties @@ -133,7 +133,7 @@ displayname.map.and.getorelse.false.to.exists=Map and getOrElse(false) to exists displayname.getorelse.null.to.ornull=GetOrElse(null) to orNull displayname.emulated.option.x=Emulated Option(x) displayname.change.to.filter=Change to filter -displayname.change.tostring.to.getorelse.on.option=Change toString to getOrElse +displayname.change.tostring.to.mkstring.on.option=Change toString to mkString displayname.some.to.option=Some to Option displayname.filter.after.sort=Filter after sort displayname.redundant.collection.conversion=Redundant collection conversion From 0939e8e091c975669dc06f2fcf2eeda5eab87603 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 11:05:52 +0200 Subject: [PATCH 07/19] refactoring of getToStringToMkStringSimplification --- .../codeInspection/collections/package.scala | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 2ef38c583ed..a7e5b292e29 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -516,36 +516,36 @@ package object collections { private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) - private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isFound: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { + private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { expr match { // TODO infix notation? - case `.toString`(array) if isFound(array) => - // array.toString - Some(replace(expr).withText(invocationText(array, mkString)).highlightFrom(array)) - case someString `+` array if isString(someString) && isFound(array) => - // "string" + array - Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) - case array `+` someString if isString(someString) && isFound(array) => - // array + "string" - Some(replace(array).withText(invocationText(array, mkString)).highlightFrom(array)) - case _ if isFound(expr) => + case `.toString`(thing) if isThing(thing) => + // thing.toString + Some(replace(expr).withText(invocationText(thing, mkString)).highlightFrom(thing)) + case someString `+` thing if isString(someString) && isThing(thing) => + // "string" + thing + Some(replace(thing).withText(invocationText(thing, mkString)).highlightFrom(thing)) + case thing `+` someString if isString(someString) && isThing(thing) => + // thing + "string" + Some(replace(thing).withText(invocationText(thing, mkString)).highlightFrom(thing)) + case _ if isThing(expr) => def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, mkString)).highlightFrom(expr) expr.getParent match { case _: ScInterpolatedStringLiteral => - // s"start $array end" + // s"start $thing end" Some(result.wrapInBlock()) case null => None case parent => parent.getParent match { case `.print`(_, args@_*) if args.contains(expr) => - // System.out.println(array) + // System.out.println(thing) Some(result) case `print`(args@_*) if args.contains(expr) => - // println(array) + // println(thing) Some(result) case _: ScInterpolatedStringLiteral => - // s"start ${array} end" + // s"start ${thing} end" Some(result) case _ => None } From 292b7e2194f5f1c6125b8a4a24c5e4ab05d908db Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 12:17:40 +0200 Subject: [PATCH 08/19] add String.format("%s", thing) --- .../scala/codeInspection/collections/package.scala | 4 ++++ .../MakeArrayToStringInspectionTest.scala | 14 ++++++++++++++ .../collections/OptionToStringInspectionTest.scala | 14 ++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index a7e5b292e29..0e0dc5cee80 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -515,6 +515,7 @@ package object collections { private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) + private val `.format` = invocation("format").from(ArraySeq("java.lang.String")) private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { expr match { @@ -544,6 +545,9 @@ package object collections { case `print`(args@_*) if args.contains(expr) => // println(thing) Some(result) + case `.format`(_, args@_*) if args.contains(expr) => + // String.format("%s", thing) + Some(result) case _: ScInterpolatedStringLiteral => // s"start ${thing} end" Some(result) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index b53954de13c..8149bfa776b 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -114,4 +114,18 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe |"test" + a.mkString("Array(", ", ", ")") """.stripMargin) } + + def testStringFormat(): Unit = { + String.format("formatted: %s", Option(1)) + doTest( + s""" + |String.format("formatted: %s", ${START}Array(1)$END) + """.stripMargin, + """ + |String.format("formatted: %s", Array(1)) + """.stripMargin, + """ + |String.format("formatted: %s", Array(1).mkString("Array(", ", ", ")")) + """.stripMargin) + } } diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 3fa12d7e9fb..a859d5cc95d 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -179,4 +179,18 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin) } + def testStringFormat(): Unit = { + String.format("formatted: %s", Option(1)) + doTest( + s""" + |String.format("formatted: %s", ${START}Option(1)$END) + """.stripMargin, + """ + |String.format("formatted: %s", Option(1)) + """.stripMargin, + """ + |String.format("formatted: %s", Option(1).mkString) + """.stripMargin) + } + } From 5d2c8ac5c835dfe853b6e387e1f6cfe24c9e6739 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 12:57:14 +0200 Subject: [PATCH 09/19] add "%s".format(thing) --- .../scala/codeInspection/collections/package.scala | 4 ++++ .../MakeArrayToStringInspectionTest.scala | 13 +++++++++++++ .../collections/OptionToStringInspectionTest.scala | 14 ++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 0e0dc5cee80..f95820f7e42 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -516,6 +516,7 @@ package object collections { private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.format` = invocation("format").from(ArraySeq("java.lang.String")) + private val `.formatStringOps` = invocation("format").from(ArraySeq("scala.collection.StringOps")) private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { expr match { @@ -548,6 +549,9 @@ package object collections { case `.format`(_, args@_*) if args.contains(expr) => // String.format("%s", thing) Some(result) + case `.formatStringOps`(_, args@_*) if args.contains(expr) => + // "%s".format(thing) + Some(result) case _: ScInterpolatedStringLiteral => // s"start ${thing} end" Some(result) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index 8149bfa776b..a833db48326 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -128,4 +128,17 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe |String.format("formatted: %s", Array(1).mkString("Array(", ", ", ")")) """.stripMargin) } + + def testFormatOnString(): Unit = { + doTest( + s""" + |"formatted: %s".format(${START}Array(1)$END) + """.stripMargin, + """ + |"formatted: %s".format(Array(1)) + """.stripMargin, + """ + |"formatted: %s".format(Array(1).mkString("Array(", ", ", ")")) + """.stripMargin) + } } diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index a859d5cc95d..3be8dd462bc 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -193,4 +193,18 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin) } + def testFormatOnString(): Unit = { + "formatted: %s".format(Option(1)) + doTest( + s""" + |"formatted: %s".format(${START}Option(1)$END) + """.stripMargin, + """ + |"formatted: %s".format(Option(1)) + """.stripMargin, + """ + |"formatted: %s".format(Option(1).mkString) + """.stripMargin) + } + } From 50d15389ec8f0a70d4f9dc0e6a5d7b8926ad3306 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 13:28:47 +0200 Subject: [PATCH 10/19] remove unused expr --- .../collections/MakeArrayToStringInspectionTest.scala | 1 - .../collections/OptionToStringInspectionTest.scala | 2 -- 2 files changed, 3 deletions(-) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index a833db48326..303b9070e7e 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -116,7 +116,6 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe } def testStringFormat(): Unit = { - String.format("formatted: %s", Option(1)) doTest( s""" |String.format("formatted: %s", ${START}Array(1)$END) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 3be8dd462bc..8c992813e08 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -180,7 +180,6 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest } def testStringFormat(): Unit = { - String.format("formatted: %s", Option(1)) doTest( s""" |String.format("formatted: %s", ${START}Option(1)$END) @@ -194,7 +193,6 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest } def testFormatOnString(): Unit = { - "formatted: %s".format(Option(1)) doTest( s""" |"formatted: %s".format(${START}Option(1)$END) From fa94d0aba7c4aa9f61682bb8c7b91f843a406e4c Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 13:39:24 +0200 Subject: [PATCH 11/19] add java.lang.StringBuilder.append(thing) and scala.collection.mutable.StringBuilder.append(thing) --- .../codeInspection/collections/package.scala | 5 +++ .../MakeArrayToStringInspectionTest.scala | 32 +++++++++++++++++++ .../OptionToStringInspectionTest.scala | 31 ++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index f95820f7e42..29baa925f3b 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -517,6 +517,7 @@ package object collections { private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.format` = invocation("format").from(ArraySeq("java.lang.String")) private val `.formatStringOps` = invocation("format").from(ArraySeq("scala.collection.StringOps")) + private val `.appendOnStringBuilder` = invocation("append").from(ArraySeq("java.lang.StringBuilder", "scala.collection.mutable.StringBuilder")) private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { expr match { @@ -552,6 +553,10 @@ package object collections { case `.formatStringOps`(_, args@_*) if args.contains(expr) => // "%s".format(thing) Some(result) + case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) => + // new java.lang.StringBuilder.append(thing) + // new scala.collection.mutable.StringBuilder.append(thing) + Some(result) case _: ScInterpolatedStringLiteral => // s"start ${thing} end" Some(result) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index 303b9070e7e..b40bb65b2d8 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -140,4 +140,36 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe |"formatted: %s".format(Array(1).mkString("Array(", ", ", ")")) """.stripMargin) } + + def testAppendOnJavaStringBuilder(): Unit = { + doTest( + s""" + |val b = new java.lang.StringBuilder + |b.append(${START}Array(1)$END) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Array(1)) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Array(1).mkString("Array(", ", ", ")")) + """.stripMargin) + } + + def testAppendOnScalaStringBuilder(): Unit = { + doTest( + s""" + |val b = new scala.collection.mutable.StringBuilder + |b.append(${START}Array(1)$END) + """.stripMargin, + """ + |val b = new scala.collection.mutable.StringBuilder + |b.append(Array(1)) + """.stripMargin, + """ + |val b = new scala.collection.mutable.StringBuilder + |b.append(Array(1).mkString("Array(", ", ", ")")) + """.stripMargin) + } } diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 8c992813e08..2a70b83f037 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -205,4 +205,35 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin) } + def testAppendOnJavaStringBuilder(): Unit = { + doTest( + s""" + |val b = new java.lang.StringBuilder + |b.append(${START}Option(1)$END) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Option(1)) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Option(1).mkString) + """.stripMargin) + } + + def testAppendOnScalaStringBuilder(): Unit = { + doTest( + s""" + |val b = new scala.collection.mutable.StringBuilder + |b.append(${START}Option(1)$END) + """.stripMargin, + """ + |val b = new scala.collection.mutable.StringBuilder + |b.append(Option(1)) + """.stripMargin, + """ + |val b = new scala.collection.mutable.StringBuilder + |b.append(Option(1).mkString) + """.stripMargin) + } } From 05d36d1de978fc336535bfce9b0b13523b281d3b Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 15:17:05 +0200 Subject: [PATCH 12/19] add "%s".formatted(thing) --- .../scala/codeInspection/collections/package.scala | 4 ++++ .../MakeArrayToStringInspectionTest.scala | 13 +++++++++++++ .../collections/OptionToStringInspectionTest.scala | 13 +++++++++++++ 3 files changed, 30 insertions(+) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 29baa925f3b..37ba85ebf8f 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -515,6 +515,7 @@ package object collections { private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) + private val `.formatted` = invocation("formatted").from(ArraySeq("scala.Predef.StringFormat")) private val `.format` = invocation("format").from(ArraySeq("java.lang.String")) private val `.formatStringOps` = invocation("format").from(ArraySeq("scala.collection.StringOps")) private val `.appendOnStringBuilder` = invocation("append").from(ArraySeq("java.lang.StringBuilder", "scala.collection.mutable.StringBuilder")) @@ -553,6 +554,9 @@ package object collections { case `.formatStringOps`(_, args@_*) if args.contains(expr) => // "%s".format(thing) Some(result) + case `.formatted`(_, args@_*) if args.contains(expr) => + // "%s".formatted(thing) + Some(result) case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) => // new java.lang.StringBuilder.append(thing) // new scala.collection.mutable.StringBuilder.append(thing) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index b40bb65b2d8..5b18573299c 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -141,6 +141,19 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe """.stripMargin) } + def testFormattedOnString(): Unit = { + doTest( + s""" + |"formatted: %s".formatted(${START}Array(1)$END) + """.stripMargin, + """ + |"formatted: %s".formatted(Array(1)) + """.stripMargin, + """ + |"formatted: %s".formatted(Array(1).mkString("Array(", ", ", ")")) + """.stripMargin) + } + def testAppendOnJavaStringBuilder(): Unit = { doTest( s""" diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 2a70b83f037..370152fcf12 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -205,6 +205,19 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin) } + def testFormattedOnString(): Unit = { + doTest( + s""" + |"formatted: %s".formatted(${START}Option(1)$END) + """.stripMargin, + """ + |"formatted: %s".formatted(Option(1)) + """.stripMargin, + """ + |"formatted: %s".formatted(Option(1).mkString) + """.stripMargin) + } + def testAppendOnJavaStringBuilder(): Unit = { doTest( s""" From 9f0bc778fce4e56bf8e254549c65fed7d8200e5e Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 15:44:33 +0200 Subject: [PATCH 13/19] merge `.format` and `.formatStringOps` --- .../plugins/scala/codeInspection/collections/package.scala | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 37ba85ebf8f..ae57a53f6fb 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -515,9 +515,8 @@ package object collections { private val `print` = unqualifed(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) private val `.print` = invocation(Set("print", "println")).from(ArraySeq("scala.Predef", "java.io.PrintStream")) - private val `.formatted` = invocation("formatted").from(ArraySeq("scala.Predef.StringFormat")) - private val `.format` = invocation("format").from(ArraySeq("java.lang.String")) - private val `.formatStringOps` = invocation("format").from(ArraySeq("scala.collection.StringOps")) + private val `.formatted` = invocation("formatted").from(ArraySeq("scala.Predef.StringFormat", "java.lang.String")) + private val `.format` = invocation("format").from(ArraySeq("java.lang.String", "scala.collection.StringOps")) private val `.appendOnStringBuilder` = invocation("append").from(ArraySeq("java.lang.StringBuilder", "scala.collection.mutable.StringBuilder")) private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { @@ -550,8 +549,6 @@ package object collections { Some(result) case `.format`(_, args@_*) if args.contains(expr) => // String.format("%s", thing) - Some(result) - case `.formatStringOps`(_, args@_*) if args.contains(expr) => // "%s".format(thing) Some(result) case `.formatted`(_, args@_*) if args.contains(expr) => From 3d2101aeec22aa95ce115f6beb0f07ada1dfaa89 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Wed, 21 Sep 2022 16:18:19 +0200 Subject: [PATCH 14/19] add thing.formatted("%s") --- .../scala/codeInspection/collections/package.scala | 3 ++- .../MakeArrayToStringInspectionTest.scala | 13 +++++++++++++ .../collections/OptionToStringInspectionTest.scala | 13 +++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index ae57a53f6fb..0298210792e 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -551,8 +551,9 @@ package object collections { // String.format("%s", thing) // "%s".format(thing) Some(result) - case `.formatted`(_, args@_*) if args.contains(expr) => + case `.formatted`(obj, args@_*) if args.contains(expr) || isThing(obj) => // "%s".formatted(thing) + // thing.formatted("%s") Some(result) case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) => // new java.lang.StringBuilder.append(thing) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index 5b18573299c..962ae55dc6f 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -154,6 +154,19 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe """.stripMargin) } + def testFormatted(): Unit = { + doTest( + s""" + |${START}Array(1)$END.formatted("formatted: %s") + """.stripMargin, + """ + |Array(1).formatted("formatted: %s") + """.stripMargin, + """ + |Array(1).mkString("Array(", ", ", ")").formatted("formatted: %s") + """.stripMargin) + } + def testAppendOnJavaStringBuilder(): Unit = { doTest( s""" diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index 370152fcf12..d99cbec5b7f 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -218,6 +218,19 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin) } + def testFormatted(): Unit = { + doTest( + s""" + |${START}Option(1)$END.formatted("formatted: %s") + """.stripMargin, + """ + |Option(1).formatted("formatted: %s") + """.stripMargin, + """ + |Option(1).mkString.formatted("formatted: %s") + """.stripMargin) + } + def testAppendOnJavaStringBuilder(): Unit = { doTest( s""" From ce86eb13ebcf778b4b85e3777ca0b75048fdcb4e Mon Sep 17 00:00:00 2001 From: cobr123 Date: Thu, 22 Sep 2022 08:24:35 +0200 Subject: [PATCH 15/19] "Format with mkString" instead of "Replace with mkString" --- .../resources/messages/ScalaInspectionBundle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties index ad0a879a253..5c5970e6383 100644 --- a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties +++ b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties @@ -309,7 +309,7 @@ fold.true.and.hint=Replace fold with forall get.getOrElse.hint=Replace with getOrElse(key, defaultValue) ### org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala -option.mkString.hint=Replace with mkString +option.mkString.hint=Format with mkString ### org/jetbrains/plugins/scala/codeInspection/collections/GetOrElseNullInspection.scala getOrElse.null.hint=Replace getOrElse(null) with orNull From 56ba55ceaf63dfd52b3b5da14a5178b28965b651 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Thu, 22 Sep 2022 11:00:11 +0200 Subject: [PATCH 16/19] fix thing.formatted("%s") --- .../plugins/scala/codeInspection/collections/package.scala | 7 +++++-- .../collections/MakeArrayToStringInspectionTest.scala | 2 +- .../collections/OptionToStringInspectionTest.scala | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 0298210792e..57ce9d9a150 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -531,6 +531,10 @@ package object collections { case thing `+` someString if isString(someString) && isThing(thing) => // thing + "string" Some(replace(thing).withText(invocationText(thing, mkString)).highlightFrom(thing)) + case thing `.formatted` someString if isString(someString) && isThing(thing) => + // thing.formatted("%s") + val thingText = thing.getText + Some(replace(expr).withText(invocationText(someString, s"format($thingText.$mkString)")).highlightElem(thing)) case _ if isThing(expr) => def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, mkString)).highlightFrom(expr) @@ -551,9 +555,8 @@ package object collections { // String.format("%s", thing) // "%s".format(thing) Some(result) - case `.formatted`(obj, args@_*) if args.contains(expr) || isThing(obj) => + case `.formatted`(_, args@_*) if args.contains(expr) => // "%s".formatted(thing) - // thing.formatted("%s") Some(result) case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) => // new java.lang.StringBuilder.append(thing) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index 962ae55dc6f..5b1bf871fc5 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -163,7 +163,7 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe |Array(1).formatted("formatted: %s") """.stripMargin, """ - |Array(1).mkString("Array(", ", ", ")").formatted("formatted: %s") + |"formatted: %s".format(Array(1).mkString("Array(", ", ", ")")) """.stripMargin) } diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index d99cbec5b7f..c5a66b6c065 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -227,7 +227,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |Option(1).formatted("formatted: %s") """.stripMargin, """ - |Option(1).mkString.formatted("formatted: %s") + |"formatted: %s".format(Option(1).mkString) """.stripMargin) } From 0d87405a34b913cde1f202e72165ce6906afa63b Mon Sep 17 00:00:00 2001 From: cobr123 Date: Thu, 22 Sep 2022 17:27:06 +0200 Subject: [PATCH 17/19] add mkString in StringBuilder.append(thing) only for append(Any) --- .../codeInspection/collections/package.scala | 3 ++- .../MakeArrayToStringInspectionTest.scala | 24 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 57ce9d9a150..809626e8737 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -558,7 +558,8 @@ package object collections { case `.formatted`(_, args@_*) if args.contains(expr) => // "%s".formatted(thing) Some(result) - case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) => + case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) + && args.exists(_.smartExpectedType().exists(_.canonicalText == "Any")) => // new java.lang.StringBuilder.append(thing) // new scala.collection.mutable.StringBuilder.append(thing) Some(result) diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala index 5b1bf871fc5..47bb13af696 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspectionTest.scala @@ -2,6 +2,8 @@ package org.jetbrains.plugins.scala package codeInspection package collections +import org.junit.Assert.assertEquals + class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTest { override protected val classOfInspection: Class[_ <: OperationOnCollectionInspection] = @@ -167,7 +169,27 @@ class MakeArrayToStringInspectionTest extends OperationsOnCollectionInspectionTe """.stripMargin) } - def testAppendOnJavaStringBuilder(): Unit = { + def testAppendNotAnyOnJavaStringBuilder(): Unit = { + try { + doTest( + s""" + |val b = new java.lang.StringBuilder + |b.append(${START}Array('1')$END) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Array('1')) + """.stripMargin, + """ + |val b = new java.lang.StringBuilder + |b.append(Array('1')) + """.stripMargin) + } catch { + case e: AssertionError => assertEquals(e.getMessage, """Highlights not found: Format with .mkString("Array(", ", ", ")")""") + } + } + + def testAppendAnyOnJavaStringBuilder(): Unit = { doTest( s""" |val b = new java.lang.StringBuilder From 1ce8a677e672d51cd43541d79fffff0fe32bcbc5 Mon Sep 17 00:00:00 2001 From: cobr123 Date: Fri, 23 Sep 2022 08:49:08 +0200 Subject: [PATCH 18/19] isAny instead of canonicalText == "Any" --- .../plugins/scala/codeInspection/collections/package.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 809626e8737..50af3b05a37 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -559,7 +559,7 @@ package object collections { // "%s".formatted(thing) Some(result) case `.appendOnStringBuilder`(_, args@_*) if args.contains(expr) - && args.exists(_.smartExpectedType().exists(_.canonicalText == "Any")) => + && args.exists(_.smartExpectedType().exists(_.isAny)) => // new java.lang.StringBuilder.append(thing) // new scala.collection.mutable.StringBuilder.append(thing) Some(result) From d4742dff08100d8357ddbd9b4d67cb7eeb9229eb Mon Sep 17 00:00:00 2001 From: cobr123 Date: Fri, 23 Sep 2022 09:33:27 +0200 Subject: [PATCH 19/19] getOrElse instead of mkString for Option --- .../META-INF/scala-plugin-common.xml | 2 +- .../OptionToString.html | 6 +-- .../messages/ScalaInspectionBundle.properties | 4 +- .../MakeArrayToStringInspection.scala | 4 +- .../OptionToStringInspection.scala | 43 +++++++++++++++-- .../codeInspection/collections/package.scala | 13 +++--- .../OptionToStringInspectionTest.scala | 46 +++++++++---------- 7 files changed, 76 insertions(+), 42 deletions(-) diff --git a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml index 35eca3d77e0..802b0d6ae14 100644 --- a/scala/scala-impl/resources/META-INF/scala-plugin-common.xml +++ b/scala/scala-impl/resources/META-INF/scala-plugin-common.xml @@ -1771,7 +1771,7 @@ enabledByDefault="true" language="Scala"/> diff --git a/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html b/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html index 9373481417f..590d8baa341 100644 --- a/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html +++ b/scala/scala-impl/resources/inspectionDescriptions/OptionToString.html @@ -1,8 +1,8 @@ -

Reports and replaces toString with mkString.

+

Reports and replaces toString with map(_.toString).getOrElse("").

-

mkString will prevent from getting string "Some(a)" instead of expected string "a".

+

map(_.toString).getOrElse("") will prevent from getting string "Some(a)" instead of expected string "a".

Example:


@@ -10,7 +10,7 @@

After the quick-fix is applied:


-  Option(a).mkString
+  Option(a).map(_.toString).getOrElse("")
diff --git a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties index 5c5970e6383..9a246503fd2 100644 --- a/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties +++ b/scala/scala-impl/resources/messages/ScalaInspectionBundle.properties @@ -133,7 +133,7 @@ displayname.map.and.getorelse.false.to.exists=Map and getOrElse(false) to exists displayname.getorelse.null.to.ornull=GetOrElse(null) to orNull displayname.emulated.option.x=Emulated Option(x) displayname.change.to.filter=Change to filter -displayname.change.tostring.to.mkstring.on.option=Change toString to mkString +displayname.change.tostring.on.option=Change toString to getOrElse("") displayname.some.to.option=Some to Option displayname.filter.after.sort=Filter after sort displayname.redundant.collection.conversion=Redundant collection conversion @@ -309,7 +309,7 @@ fold.true.and.hint=Replace fold with forall get.getOrElse.hint=Replace with getOrElse(key, defaultValue) ### org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala -option.mkString.hint=Format with mkString +option.toString.hint=Format with map(_.toString).getOrElse("") ### org/jetbrains/plugins/scala/codeInspection/collections/GetOrElseNullInspection.scala getOrElse.null.hint=Replace getOrElse(null) with orNull diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala index 7527756e773..d8e994066cd 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/MakeArrayToStringInspection.scala @@ -13,8 +13,8 @@ class MakeArrayToStringInspection extends OperationOnCollectionInspection { object MakeArrayToStringInspection extends SimplificationType { override def hint: String = ScalaInspectionBundle.message("format.with.mkstring") - private val mkString = """mkString("Array(", ", ", ")")""" + private def mkString(arr: ScExpression): String = """mkString("Array(", ", ", ")")""" override def getSimplification(expr: ScExpression): Option[Simplification] = - getToStringToMkStringSimplification(expr, isArray, mkString, replace) + getToStringSimplification(expr, isArray, mkString, replace) } diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala index fb22fd1ce4d..449cb92f101 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspection.scala @@ -2,6 +2,8 @@ package org.jetbrains.plugins.scala.codeInspection.collections import org.jetbrains.plugins.scala.codeInspection.ScalaInspectionBundle import org.jetbrains.plugins.scala.lang.psi.api.expr.ScExpression +import org.jetbrains.plugins.scala.lang.psi.types.ScParameterizedType +import org.jetbrains.plugins.scala.lang.psi.types.api.designator.{ScDesignatorType, ScProjectionType} import scala.collection.immutable.ArraySeq @@ -11,10 +13,41 @@ class OptionToStringInspection extends OperationOnCollectionInspection { object OptionToStringInspection extends SimplificationType { - override def hint: String = ScalaInspectionBundle.message("option.mkString.hint") - - private val mkString = """mkString""" + override def hint: String = ScalaInspectionBundle.message("option.toString.hint") override def getSimplification(expr: ScExpression): Option[Simplification] = - getToStringToMkStringSimplification(expr, isOption, mkString, replace) -} \ No newline at end of file + getToStringSimplification(expr, isOption, mkString, replace) + + private def mkString(option: ScExpression): String = { + option match { + case scalaNone() => """getOrElse("null")""" + case _ => + option.getTypeAfterImplicitConversion().tr match { + case Right(t: ScParameterizedType) if isStringInOption(t) => onString + case Right(t: ScDesignatorType) if isStringInOption(t) => onString + case Right(t: ScProjectionType) if isStringInOption(t) => onString + case _ => onNotString + } + } + } + + private def isStringInOption(t: ScParameterizedType): Boolean = t.typeArguments.forall(SizeToLength.isString) + + private def isStringInOption(t: ScDesignatorType): Boolean = { + t.extractDesignatorSingleton match { + case Some(t: ScParameterizedType) if isStringInOption(t) => true + case _ => false + } + } + + private def isStringInOption(t: ScProjectionType): Boolean = { + t.extractDesignatorSingleton match { + case Some(t: ScParameterizedType) if isStringInOption(t) => true + case _ => false + } + } + + private def onString: String = """getOrElse("")""" + + private def onNotString: String = """map(_.toString).getOrElse("")""" +} diff --git a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala index 50af3b05a37..8d568d4baa0 100644 --- a/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala +++ b/scala/scala-impl/src/org/jetbrains/plugins/scala/codeInspection/collections/package.scala @@ -519,24 +519,25 @@ package object collections { private val `.format` = invocation("format").from(ArraySeq("java.lang.String", "scala.collection.StringOps")) private val `.appendOnStringBuilder` = invocation("append").from(ArraySeq("java.lang.StringBuilder", "scala.collection.mutable.StringBuilder")) - private[collections] def getToStringToMkStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, mkString: String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { + private[collections] def getToStringSimplification(expr: ScExpression, isThing: ScExpression => Boolean, thingToString: ScExpression => String, replace: ScExpression => SimplificationBuilder): Option[Simplification] = { expr match { // TODO infix notation? case `.toString`(thing) if isThing(thing) => // thing.toString - Some(replace(expr).withText(invocationText(thing, mkString)).highlightFrom(thing)) + Some(replace(expr).withText(invocationText(thing, thingToString(thing))).highlightFrom(thing)) case someString `+` thing if isString(someString) && isThing(thing) => // "string" + thing - Some(replace(thing).withText(invocationText(thing, mkString)).highlightFrom(thing)) + Some(replace(thing).withText(invocationText(thing, thingToString(thing))).highlightFrom(thing)) case thing `+` someString if isString(someString) && isThing(thing) => // thing + "string" - Some(replace(thing).withText(invocationText(thing, mkString)).highlightFrom(thing)) + Some(replace(thing).withText(invocationText(thing, thingToString(thing))).highlightFrom(thing)) case thing `.formatted` someString if isString(someString) && isThing(thing) => // thing.formatted("%s") val thingText = thing.getText - Some(replace(expr).withText(invocationText(someString, s"format($thingText.$mkString)")).highlightElem(thing)) + val toStringThing = thingToString(thing) + Some(replace(expr).withText(invocationText(someString, s"format($thingText.$toStringThing)")).highlightElem(thing)) case _ if isThing(expr) => - def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, mkString)).highlightFrom(expr) + def result: SimplificationBuilder = replace(expr).withText(invocationText(expr, thingToString(expr))).highlightFrom(expr) expr.getParent match { case _: ScInterpolatedStringLiteral => diff --git a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala index c5a66b6c065..9ce9f188c39 100644 --- a/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala +++ b/scala/scala-impl/test/org/jetbrains/plugins/scala/codeInspection/collections/OptionToStringInspectionTest.scala @@ -7,13 +7,13 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest override protected val classOfInspection: Class[_ <: OperationOnCollectionInspection] = classOf[OptionToStringInspection] - override protected val hint: String = ScalaInspectionBundle.message("option.mkString.hint") + override protected val hint: String = ScalaInspectionBundle.message("option.toString.hint") def testNone(): Unit = { doTest( s"""None.${START}toString$END""", """None.toString""", - """None.mkString""" + """None.getOrElse("null")""" ) } @@ -21,7 +21,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""val i = Option("hello"); i.${START}toString$END""", """val i = Option("hello"); i.toString""", - """val i = Option("hello"); i.mkString""" + """val i = Option("hello"); i.getOrElse("")""" ) } @@ -29,7 +29,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""object Test { val i = Option("hello"); i.${START}toString$END }""", """object Test { val i = Option("hello"); i.toString }""", - """object Test { val i = Option("hello"); i.mkString }""" + """object Test { val i = Option("hello"); i.getOrElse("") }""" ) } @@ -37,7 +37,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""sys.env.get("VARIABLE").${START}toString$END""", """sys.env.get("VARIABLE").toString""", - """sys.env.get("VARIABLE").mkString""" + """sys.env.get("VARIABLE").getOrElse("")""" ) } @@ -45,7 +45,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Some("constant").${START}toString$END""", """Some("constant").toString""", - """Some("constant").mkString""" + """Some("constant").getOrElse("")""" ) } @@ -53,7 +53,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Option("constant").${START}toString$END""", """Option("constant").toString""", - """Option("constant").mkString""" + """Option("constant").getOrElse("")""" ) } @@ -61,7 +61,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""val i = Option(1); i.${START}toString$END""", """val i = Option(1); i.toString""", - """val i = Option(1); i.mkString""" + """val i = Option(1); i.map(_.toString).getOrElse("")""" ) } @@ -69,7 +69,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""def getSomeOne():Option[Int] = Some(1); getSomeOne().${START}toString$END""", """def getSomeOne():Option[Int] = Some(1); getSomeOne().toString""", - """def getSomeOne():Option[Int] = Some(1); getSomeOne().mkString""" + """def getSomeOne():Option[Int] = Some(1); getSomeOne().map(_.toString).getOrElse("")""" ) } @@ -77,7 +77,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Some(1).${START}toString$END""", """Some(1).toString""", - """Some(1).mkString""" + """Some(1).map(_.toString).getOrElse("")""" ) } @@ -85,7 +85,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest doTest( s"""Option(1).${START}toString$END""", """Option(1).toString""", - """Option(1).mkString""" + """Option(1).map(_.toString).getOrElse("")""" ) } @@ -98,7 +98,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |println(Option(3)) """.stripMargin, """ - |println(Option(3).mkString) + |println(Option(3).map(_.toString).getOrElse("")) """.stripMargin) } @@ -111,7 +111,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |System.out.println(Option(3)) """.stripMargin, """ - |System.out.println(Option(3).mkString) + |System.out.println(Option(3).map(_.toString).getOrElse("")) """.stripMargin) } @@ -127,7 +127,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val a = Option(3) - |s"before ${a.mkString} after" + |s"before ${a.map(_.toString).getOrElse("")} after" """.stripMargin) } @@ -143,7 +143,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val a = Option(3) - |s"before ${a.mkString} after" + |s"before ${a.map(_.toString).getOrElse("")} after" """.stripMargin) } @@ -159,7 +159,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val a = Option(3) - |a.mkString + "test" + |a.map(_.toString).getOrElse("") + "test" """.stripMargin) } @@ -175,7 +175,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val a = Option(3) - |"test" + a.mkString + |"test" + a.map(_.toString).getOrElse("") """.stripMargin) } @@ -188,7 +188,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |String.format("formatted: %s", Option(1)) """.stripMargin, """ - |String.format("formatted: %s", Option(1).mkString) + |String.format("formatted: %s", Option(1).map(_.toString).getOrElse("")) """.stripMargin) } @@ -201,7 +201,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |"formatted: %s".format(Option(1)) """.stripMargin, """ - |"formatted: %s".format(Option(1).mkString) + |"formatted: %s".format(Option(1).map(_.toString).getOrElse("")) """.stripMargin) } @@ -214,7 +214,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |"formatted: %s".formatted(Option(1)) """.stripMargin, """ - |"formatted: %s".formatted(Option(1).mkString) + |"formatted: %s".formatted(Option(1).map(_.toString).getOrElse("")) """.stripMargin) } @@ -227,7 +227,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest |Option(1).formatted("formatted: %s") """.stripMargin, """ - |"formatted: %s".format(Option(1).mkString) + |"formatted: %s".format(Option(1).map(_.toString).getOrElse("")) """.stripMargin) } @@ -243,7 +243,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val b = new java.lang.StringBuilder - |b.append(Option(1).mkString) + |b.append(Option(1).map(_.toString).getOrElse("")) """.stripMargin) } @@ -259,7 +259,7 @@ class OptionToStringInspectionTest extends OperationsOnCollectionInspectionTest """.stripMargin, """ |val b = new scala.collection.mutable.StringBuilder - |b.append(Option(1).mkString) + |b.append(Option(1).map(_.toString).getOrElse("")) """.stripMargin) } }