Skip to content
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.

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

Remove workarounds for Scala 3 error message checks in tests #354

Merged
merged 11 commits into from
Sep 11, 2023

Conversation

MateuszKubuszok
Copy link
Member

@MateuszKubuszok MateuszKubuszok commented Aug 29, 2023

TODO:

  • manually align the behavior of Type.prettyPrint[T] between Scala 2 and Scala 3
  • fix methods generating Transformer and PartialTransformer type classes in Scala 3 - currently the pass src as the name of the expr, while they should use fresh name based on type
    • ditto for Patcher
  • manually drop the suffix $macro$1 from some freshterms on Scala 3 to align the behavior with Scala 2
    • or better - change Scala 2 to add $macro to prefix so that both implementations would generate $prefix$macro$n, remove special cases in dropping $macro$n and add this suffix in tests
  • await fixing Incorrect children queried before class was discovered error when typeCheckErrors is used scala/scala3#18484

@MateuszKubuszok
Copy link
Member Author

MateuszKubuszok commented Aug 29, 2023

Blocked by a bug in scala.compiletime.testing.typeCheckErrors, reproduced in https://gist.github.com/MateuszKubuszok/055cef0b01a24fa430ff8aaf8a2caca0

@MateuszKubuszok MateuszKubuszok linked an issue Aug 29, 2023 that may be closed by this pull request
3 tasks
@MateuszKubuszok
Copy link
Member Author

After some discussions and investigations, it seems that it's not a bug but an intended behavior. To fix all tests and have the same error messages between 2 and 3 I need to:

  • manually align the behavior of Type.prettyPrint[T] between Scala 2 and Scala 3
  • fix methods generating Transformer and PartialTransformer type classes in Scala 3 - currently the pass src as the name of the expr, while they should use fresh name based on type
  • manually drop the suffix $macro$1 from some freshterms on Scala 3 to align the behavior with Scala 2

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 82.05% and project coverage change: -0.09% ⚠️

Comparison is base (ac3d2a0) 79.11% compared to head (fc2f654) 79.02%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #354      +/-   ##
==========================================
- Coverage   79.11%   79.02%   -0.09%     
==========================================
  Files         105      105              
  Lines        3974     4020      +46     
  Branches      177      179       +2     
==========================================
+ Hits         3144     3177      +33     
- Misses        830      843      +13     
Files Changed Coverage Δ
...ey/internal/compiletime/ChimneyExprsPlatform.scala 81.69% <44.44%> (-7.38%) ⬇️
...himney/internal/compiletime/DerivationResult.scala 72.37% <71.42%> (-0.04%) ⬇️
...ey/internal/compiletime/ExprPromisesPlatform.scala 96.96% <100.00%> (+0.14%) ⬆️
...d/chimney/internal/compiletime/ExprsPlatform.scala 88.52% <100.00%> (-2.22%) ⬇️
...d/chimney/internal/compiletime/TypesPlatform.scala 83.33% <100.00%> (-5.20%) ⬇️
...ernal/compiletime/derivation/patcher/Gateway.scala 100.00% <100.00%> (ø)
...ompiletime/derivation/transformer/Derivation.scala 87.50% <100.00%> (+4.16%) ⬆️
...l/compiletime/derivation/transformer/Gateway.scala 100.00% <100.00%> (ø)
...rmSealedHierarchyToSealedHierarchyRuleModule.scala 69.29% <100.00%> (+2.04%) ⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MateuszKubuszok MateuszKubuszok marked this pull request as ready for review September 11, 2023 14:45
@MateuszKubuszok MateuszKubuszok merged commit 488f453 into master Sep 11, 2023
20 of 21 checks passed
@MateuszKubuszok MateuszKubuszok deleted the run-scala-3-errors-checks branch September 11, 2023 15:58
@MateuszKubuszok
Copy link
Member Author

Ultimately, showing $macro$1 in outputs would make things harder to tests, maybe in the future we will remove it only from tests messages, but for now we remove it always from macro logs. Additionally, I re-ignored 1 test on Scala 3 which fails due to a compiler bug scala/scala3#18484 . Since these improvements will be helpful in other work needed for 0.8.0 I merge this and the last test will be un-ignored when fix will be available.

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.

Fix Scala 3's error message format
1 participant