Content-Length: 395477 | pFad | https://github.com/scala/scala/pull/10776

53 Add `-Wtostring-interpolated` to warn if interpolator uses `toString` by som-snytt · Pull Request #10776 · scala/scala · GitHub
Skip to content

Add -Wtostring-interpolated to warn if interpolator uses toString #10776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 20, 2024
Merged

Add -Wtostring-interpolated to warn if interpolator uses toString #10776

merged 1 commit into from
Jun 20, 2024

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented May 7, 2024 •
edited
Loading

Adds -Wtostring-interpolated which warns if a standard interpolator uses toString to convert a value which is not string or primitive type.

-Wconf:cat=w-flag-tostring-interpolated:e to boost.

Also restores edge case f"${ "hello" }%b" which is "true" for non-boolean arg. That became an error in previous refactor, possibly intentionally. (It is arguably an obscure feature of format.) (Edit: the C.I. error reminds me that the change was to make the expected type Boolean, so that conversions work normally. That is also edge casey. Maybe it can try boolean, null, and any? Edit: yes, try non-any first, then any if allowed.)

The "default" %s is improved so that the warning position is at the interpolated element in s"$x".

Follow-up to #8654

@scala-jenkins scala-jenkins added this to the 2.13.15 milestone May 7, 2024
@xuwei-k xuwei-k mentioned this pull request May 8, 2024
Open
@som-snytt som-snytt force-pushed the topic/warn-toString-interpolated branch 2 times, most recently from 6e463fb to ab4a1d1 Compare May 8, 2024 06:05
@som-snytt
Copy link
Contributor Author

Previous feedback at wartremover is to allow at least CharSequence and perhaps primitives with "canonical" representations (Int, Char, but not Float, Double). What about usual formatting of Int such as 1,024?

Therefore, no accommodation is made yet here.

A future version can take a typeclass, implicit locale, etc.

@som-snytt som-snytt marked this pull request as ready for review May 8, 2024 16:59
@som-snytt som-snytt force-pushed the topic/warn-toString-interpolated branch from ab4a1d1 to cbf369d Compare May 8, 2024 17:13
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label May 21, 2024
@som-snytt som-snytt force-pushed the topic/warn-toString-interpolated branch from cbf369d to dc21f58 Compare June 16, 2024 13:05
@som-snytt
Copy link
Contributor Author

Added the exclusion for primitives but not CharSequence.

Dotty reflect says don't use toString! I will forward port.

CharSequence is indeed a sequence of chars, but the implementation could have arbitrary characteristics.

StringBuilder is a close call, but arguably you shouldn't be interpolating it but building with it.

@som-snytt som-snytt requested a review from lrytz June 16, 2024 13:28
@som-snytt som-snytt mentioned this pull request Jun 17, 2024
Merged
@som-snytt
Copy link
Contributor Author

spurious error on first commit after rebase

[info] [info] set current project to sbt_8abf1597 (in build file:/tmp/sbt_8abf1597/)
[info] [error] lmcoursier.internal.shaded.coursier.error.FetchError$DownloadingArtifacts: Error fetching artifacts:
[info] [error] file://github.com/home/jenkins/.ivy2/local/org.scala-lang/scala-compiler/2.13.15-bin-SNAPSHOT/jars/scala-compiler.jar: wrong checksum: /home/jenkins/.ivy2/local/org.scala-lang/scala-compiler/2.13.15-bin-SNAPSHOT/jars/scala-compiler.jar (expected SHA-1 83c122d8bc099f9087008091b008e29b99e89b68 in /home/jenkins/.ivy2/local/org.scala-lang/scala-compiler/2.13.15-bin-SNAPSHOT/jars/scala-compiler.jar.sha1, got 87e3f962badf079fa40348f5e4ffa1c25fe6f85d)
[info] [error] 
[info] [error] 	at lmcoursier.internal.shaded.coursier.Artifacts$.$anonfun$fetchArtifacts$9(Artifacts.scala:365)
[info] [error] 	at lmcoursier.internal.shaded.coursier.util.Task$.$anonfun$flatMap$extension$1(Task.scala:14)
[info] [error] 	at lmcoursier.internal.shaded.coursier.util.Task$.$anonfun$flatMap$extension$1$adapted(Task.scala:14)
[info] [error] 	at lmcoursier.internal.shaded.coursier.util.Task$.wrap(Task.scala:82)
[info] [error] 	at lmcoursier.internal.shaded.coursier.util.Task$.$anonfun$flatMap$2(Task.scala:14)
[info] [error] 	at scala.concurrent.Future.$anonfun$flatMap$1(Future.scala:307)
[info] [error] 	at scala.concurrent.impl.Promise.$anonfun$transformWith$1(Promise.scala:51)
[info] [error] 	at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:74)
[info] [error] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] [error] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] [error] 	at java.lang.Thread.run(Thread.java:750)
[info] [error] Caused by: lmcoursier.internal.shaded.coursier.cache.ArtifactError$WrongChecksum: wrong checksum: /home/jenkins/.ivy2/local/org.scala-lang/scala-compiler/2.13.15-bin-SNAPSHOT/jars/scala-compiler.jar (expected SHA-1 83c122d8bc099f9087008091b008e29b99e89b68 in /home/jenkins/.ivy2/local/org.scala-lang/scala-compiler/2.13.15-bin-SNAPSHOT/jars/scala-compiler.jar.sha1, got 87e3f962badf079fa40348f5e4ffa1c25fe6f85d)
[info] [error] 	at lmcoursier.internal.shaded.coursier.cache.FileCache.$anonfun$validateChecksum$4(FileCache.scala:146)
[info] [error] 	at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:659)
[info] [error] 	at scala.util.Success.$anonfun$map$1(Try.scala:255)
[info] [error] 	at scala.util.Success.map(Try.scala:213)
[info] [error] 	at scala.concurrent.Future.$anonfun$map$1(Future.scala:292)
[info] [error] 	at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:42)
[info] [error] 	at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:74)
[info] [error] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] [error] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] [error] 	at java.lang.Thread.run(Thread.java:750)
[info] [error] (update) lmcoursier.internal.shaded.coursier.error.FetchError$DownloadingArtifacts: Error fetching artifacts:

then

[error] [info] waiting for lock on /home/jenkins/.ivy2/.sbt.ivy.lock to be available...
[info] [info] compiling 1 Scala source to /tmp/sbt_8131f52f/macro-provider/target/classes ...
[info] [error] ## Exception when compiling 1 sources to /tmp/sbt_8131f52f/macro-provider/target/classes
[info] [error] java.lang.NoClassDefFoundError: scala/reflect/macros/internal/macroImpl
[info] [error] scala.reflect.internal.Definitions$DefinitionsClass.MacroImplAnnotation$lzycompute(Definitions.scala:587)
[info] [error] scala.reflect.internal.Definitions$DefinitionsClass.MacroImplAnnotation(Definitions.scala:587)
[info] [error] scala.tools.nsc.typechecker.Macros.$anonfun$loadMacroImplBinding$1(Macros.scala:291)
[info] [error] scala.collection.mutable.AnyRefMap.getOrElseUpdate(AnyRefMap.scala:169)
[info] [error] scala.tools.nsc.typechecker.Macros.loadMacroImplBinding(Macros.scala:293)
[info] [error] scala.tools.nsc.typechecker.Macros.loadMacroImplBinding$(Macros.scala:289)
[info] [error] scala.tools.nsc.Global$$anon$6.loadMacroImplBinding(Global.scala:514)
[info] [error] scala.tools.nsc.typechecker.Macros.standardIsBlackbox(Macros.scala:310)
[info] [error] scala.tools.nsc.typechecker.Macros.standardIsBlackbox$(Macros.scala:308)
[info] [error] scala.tools.nsc.Global$$anon$6.standardIsBlackbox(Global.scala:514)
[info] [error] scala.tools.nsc.typechecker.AnalyzerPlugins$$anon$10.default(AnalyzerPlugins.scala:466)
[info] [error] scala.tools.nsc.typechecker.AnalyzerPlugins$$anon$10.default(AnalyzerPlugins.scala:463)
[info] [error] scala.tools.nsc.typechecker.AnalyzerPlugins.invoke(AnalyzerPlugins.scala:428)

lrytz
lrytz reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

LGTM otherwise! I'm not familiar with the code though.

@som-snytt som-snytt force-pushed the topic/warn-toString-interpolated branch from dc21f58 to 6afc3a5 Compare June 17, 2024 18:01
@som-snytt
Copy link
Contributor Author

gah

[info] [info] compiling 2 Scala sources to /tmp/sbt_9b694d86/target/classes ...
[info] [error] ## Exception when compiling 2 sources to /tmp/sbt_9b694d86/target/classes
[info] [error] java.util.ServiceConfigurationError: xsbti.compile.CompilerInterface2: Error reading configuration file
[info] [error] java.util.ServiceLoader.fail(ServiceLoader.java:232)

@som-snytt som-snytt mentioned this pull request Jun 17, 2024
Closed
@som-snytt
Copy link
Contributor Author

/rebuild

@som-snytt
Copy link
Contributor Author

fool me twice

[info] [info] compiling 1 Scala source to /tmp/sbt_f26212df/target/classes ...
[info] [error] ## Exception when compiling 2 sources to /tmp/sbt_f26212df/target/classes
[info] [error] java.util.ServiceConfigurationError: xsbti.compile.CompilerInterface2: Error reading configuration file
[info] [error] java.util.ServiceLoader.fail(ServiceLoader.java:232)

lrytz
lrytz approved these changes Jun 18, 2024
View reviewed changes
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Nice, thank you! Let's squash before merge.

@som-snytt som-snytt force-pushed the topic/warn-toString-interpolated branch from d138e6c to 1ac2451 Compare June 19, 2024 20:16
@som-snytt
Copy link
Contributor Author

Squashed & rebased for ✔️ (🤞 )

@som-snytt som-snytt merged commit 1bda625 into scala:2.13.x Jun 20, 2024
3 checks passed
@som-snytt som-snytt deleted the topic/warn-toString-interpolated branch June 20, 2024 00:51
@SethTisue SethTisue changed the title Warn if interpolator uses toString Add Adds -Wtostring-interpolated to warn if interpolator uses toString Aug 22, 2024
@SethTisue SethTisue changed the title Add Adds -Wtostring-interpolated to warn if interpolator uses toString Add -Wtostring-interpolated to warn if interpolator uses toString Aug 22, 2024
@SethTisue SethTisue mentioned this pull request Mar 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrytz lrytz lrytz approved these changes

Assignees
No one assigned
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.13.15
Development

Successfully merging this pull request may close these issues.
4 participants
@som-snytt @lrytz @SethTisue @scala-jenkins








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/scala/scala/pull/10776

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy