Skip to content

Conversation

joroKr21
Copy link
Member

@joroKr21 joroKr21 commented Dec 29, 2023

Fix #7092

@joroKr21 joroKr21 force-pushed the implicit-not-found branch 5 times, most recently from c550fdc to 06f086b Compare December 29, 2023 07:19
@joroKr21 joroKr21 marked this pull request as ready for review December 29, 2023 08:08
FileDiff.dump(checkFile.toPath.toString, actual)
echo("Updated checkfile: " + checkFile.getPath)
} else {
onFailure(testSource, reporters, logger, Some(msg))
Copy link
Contributor

Choose a reason for hiding this comment

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

Originally I left this one out to force users to re-run the tests against the new checkfile. This can be useful if there is a source of non-determinism in the output.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can revert this. Was pretty confusing initially but I might be biased by the Scala 2 experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can keep your change. There is no right or wrong way to do this. Your way might be easier to understand.

@nicolasstucki nicolasstucki requested review from mbovel and removed request for anatoliykmetyuk February 1, 2024 09:13
@joroKr21
Copy link
Member Author

Any chance to get a review? I'm not sure who to ping. I guess the 3.2 ship has sailed, but I hope we can backport it to 3.3

Copy link
Member

@mbovel mbovel left a comment

Choose a reason for hiding this comment

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

Just two minor test/doc comments, otherwise looks good to me.


/** @param rawMsg Message template with variables, e.g. "Variable A is ${A}"
* @param sym Symbol of the annotated type or of the method whose parameter was annotated
* @param substituteType Function substituting specific types for abstract types associated with variables, e.g A -> Int
Copy link
Member

Choose a reason for hiding this comment

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

The documentation should be updated to include the new parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍

if (idx >= 0) Some(i"${args(idx)}") else None
"""\$\{\s*([^}\s]+)\s*\}""".r.replaceAllIn(raw, (_: Regex.Match) match
case Regex.Groups(v) => quoteReplacement(translate(v).getOrElse("")).nn
case Regex.Groups(v) => quoteReplacement(translate(v).getOrElse("?" + v)).nn
Copy link
Member

Choose a reason for hiding this comment

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

We should maybe have a separate test case for the "?" case?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have this test case:

26 |  summon[H[Int]] // error
   |                ^
   |                Not found for Int, ?B

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I wrote a separate test case because I think it could deserve its own tests, but that's okay like that.

@joroKr21 joroKr21 force-pushed the implicit-not-found branch from 06f086b to d34ab6d Compare April 20, 2024 17:28
@mbovel mbovel merged commit 7c67d7c into scala:main Apr 25, 2024
@joroKr21 joroKr21 deleted the implicit-not-found branch April 25, 2024 08:10
joroKr21 added a commit to joroKr21/dotty that referenced this pull request Apr 27, 2024
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
WojciechMazur added a commit that referenced this pull request Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@implicitNotFound string interpolator doesn't work with types

4 participants