[Issue 12] `@unroll` on final or effectively final methods by arainko · Pull Request #15 · com-lihaoyi/unroll

arainko · 2025-09-02T05:17:25Z

The Scala 3 version of the plugin now closely follows the various checks introduced in the SIP implementation (with the actual impl being a mix of these 3 diffs: diff 1, diff 2, diff 3).

For the Scala 2 version I had to get a tad bit more creative since it lacked (or I just couldn't find the correct combinations of methods, lol) built-in checks for the likes of a method being local (which is now checked by iterating over the owners and checking if any of them is a method) or it had straight up different behavior compared to dotty (e.g. constructors not being considered effectively final).

All of the original test cases (with the exception of abstractClassMethod and abstractTraitMethod which had to be converted into negative tests) work after annotating their respective classes (or just the methods that were there before) with final.

There's also the aforementioned negative module which has two variants - a Scala 3 and a Scala 2 one (the only difference between those being an additional check for rejecting trait constructors on the Scala 3 side). This module is not automatically checked in any shape or form (pretty much functions as an 'eyeball' test for whoever looks at the code).
FTR these are the compilation errors being reported at the time of writing this PR:

Scala 3

aleksander@pop-os:~/repos/unroll-1$ ./mill -i -k "unroll[3.7.1].negative.compile"
Another Mill process is running 'IDE:buildTargetDependencyModules', waiting for it to be done...
[156/156] unroll[3.7.1].negative.compile
[156] [info] compiling 1 Scala source to /home/aleksander/repos/unroll-1/out/unroll/3.7.1/negative/compile.dest/classes ...
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:6:28 
[156] [error] 6 |  def foo(int: Int, @unroll str: String = "1") = int.toString() + str
[156] [error]   |                            ^
[156] [error]   |       Cannot unroll parameters of method foo because it can be overridden
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:9:8 
[156] [error]  9 |    def inner(a: Int, @unroll str: String = "1") = {
[156] [error]    |    ^
[156] [error]    |   Cannot unroll parameters of method inner because it is a local method
[156] [error] 10 |      def anEvenMoreInnerMethUnrolled(@unroll int: Int = 1) = ()
[156] [error] 11 |      str
[156] [error] 12 |    }
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:18:29 
[156] [error] 18 |  def foo(s: String, @unroll n: Int = 1): String
[156] [error]    |                             ^
[156] [error]    |     Cannot unroll parameters of method foo because it can be overridden
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:22:29 
[156] [error] 22 |  def foo(s: String, @unroll n: Int = 1): String
[156] [error]    |                             ^
[156] [error]    |     Cannot unroll parameters of method foo because it can be overridden
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:21:27 
[156] [error] 21 |trait FooTrait(@unroll val param: Int = 1, @unroll val param2: String = "asd") {
[156] [error]    |                           ^
[156] [error]    |implementation restriction: Cannot unroll parameters of a trait constructor
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:21:55 
[156] [error] 21 |trait FooTrait(@unroll val param: Int = 1, @unroll val param2: String = "asd") {
[156] [error]    |                                                       ^
[156] [error]    |implementation restriction: Cannot unroll parameters of a trait constructor
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:26:12 
[156] [error] 26 |  final def copy(int: Int = int, @unroll str: String = str): FooCaseClass = FooCaseClass(int, str)
[156] [error]    |            ^
[156] [error]    |Cannot unroll parameters of method copy of a case class: please annotate the class constructor instead
[156] [error] -- Error: /home/aleksander/repos/unroll-1/unroll/negative/scala3/src/invalidunroll.scala:30:6 
[156] [error] 30 |  def apply(int: Int, @unroll str: String): FooCaseClass = FooCaseClass(int, str)
[156] [error]    |      ^
[156] [error]    |Cannot unroll parameters of method apply of a case class companion object: please annotate the class constructor instead
[156] [error] 8 errors found
[156] unroll[3.7.1].negative.compile failed
[156/156, 1 failed] ============================== unroll[3.7.1].negative.compile ============================== 9s
1 tasks failed
unroll[3.7.1].negative.compile Compilation failed

Scala 2.13

aleksander@pop-os:~/repos/unroll-1$ ./mill -i -k "unroll[2.13.12].negative.compile"
Another Mill process is running 'IDE:buildTargetJavacOptions', waiting for it to be done...
[156/156] unroll[2.13.12].negative.compile
[156] [info] compiling 1 Scala source to /home/aleksander/repos/unroll-1/out/unroll/2.13.12/negative/compile.dest/classes ...
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:6:29: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(int: Int, @unroll str: String = "1") = int.toString() + str
[156] [error]                             ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:9:9: Cannot unroll parameters of method inner because it is a local method
[156] [error]     def inner(a: Int, @unroll str: String = "1") = {
[156] [error]         ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:10:11: Cannot unroll parameters of method anEvenMoreInnerMethUnrolled because it is a local method
[156] [error]       def anEvenMoreInnerMethUnrolled(@unroll int: Int = 1) = ()
[156] [error]           ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:18:30: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(s: String, @unroll n: Int = 1): String
[156] [error]                              ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:22:30: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(s: String, @unroll n: Int = 1): String
[156] [error]                              ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:26:42: Cannot unroll parameters of method copy of a case class companion object: please annotate the class constructor instead
[156] [error]   final def copy(int: Int = int, @unroll str: String = str): FooCaseClass = FooCaseClass(int, str)
[156] [error]                                          ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:30:31: Cannot unroll parameters of method apply of a case class companion object: please annotate the class constructor instead
[156] [error]   def apply(int: Int, @unroll str: String): FooCaseClass = FooCaseClass(int, str)
[156] [error]                               ^
[156] [error] 7 errors found
[156] unroll[2.13.12].negative.compile failed
[156/156, 1 failed] ============================== unroll[2.13.12].negative.compile ============================== 5s
1 tasks failed
unroll[2.13.12].negative.compile Compilation failed

Scala 2.12

aleksander@pop-os:~/repos/unroll-1$ ./mill -i -k "unroll[2.12.18].negative.compile"
[156/156] unroll[2.12.18].negative.compile
[156] [info] compiling 1 Scala source to /home/aleksander/repos/unroll-1/out/unroll/2.12.18/negative/compile.dest/classes ...
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:6:29: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(int: Int, @unroll str: String = "1") = int.toString() + str
[156] [error]                             ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:9:9: Cannot unroll parameters of method inner because it is a local method
[156] [error]     def inner(a: Int, @unroll str: String = "1") = {
[156] [error]         ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:10:11: Cannot unroll parameters of method anEvenMoreInnerMethUnrolled because it is a local method
[156] [error]       def anEvenMoreInnerMethUnrolled(@unroll int: Int = 1) = ()
[156] [error]           ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:18:30: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(s: String, @unroll n: Int = 1): String
[156] [error]                              ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:22:30: Cannot unroll parameters of method foo because it can be overridden
[156] [error]   def foo(s: String, @unroll n: Int = 1): String
[156] [error]                              ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:26:42: Cannot unroll parameters of method copy of a case class companion object: please annotate the class constructor instead
[156] [error]   final def copy(int: Int = int, @unroll str: String = str): FooCaseClass = FooCaseClass(int, str)
[156] [error]                                          ^
[156] [error] /home/aleksander/repos/unroll-1/unroll/negative/scala2/src/invalidunroll.scala:30:31: Cannot unroll parameters of method apply of a case class companion object: please annotate the class constructor instead
[156] [error]   def apply(int: Int, @unroll str: String): FooCaseClass = FooCaseClass(int, str)
[156] [error]                               ^
[156] [error] 7 errors found
[156] unroll[2.12.18].negative.compile failed
[156/156, 1 failed] ============================== unroll[2.12.18].negative.compile ============================== 6s
1 tasks failed
unroll[2.12.18].negative.compile Compilation failed

Fixes #12

lihaoyi · 2025-09-02T09:23:05Z

@arainko those tests should be converted into negative tests: we should assert the failure and provide an appropriate error message. We should also ensure @unroll on non-final defs on non-final classes to make sure they fail as well with a proper error message

arainko · 2025-09-02T14:14:20Z

So before I go on an adventure: do you have a goto mechanism for testing compilation failures? There's scala.compiletime.testing.typeCheckErrors for Scala 3 but it doesn't seem to take plugins into consideration (Scala 2 has Universe#typecheck which could be resurfaced as a macro as well but I suppose it also doesn't care about plugins?). Or am I on a path to directly invoking the published compiler JAR? 😄

lihaoyi · 2025-09-02T14:27:48Z

uTest has assertCompileError, but this repo doesn't use it. I think for now we can just run __.test with --keep-going and eyeball the ones that fail to make sure it's the right ones with the right error

…gative` module

arainko · 2025-09-07T08:54:54Z

unroll/negative/scala2/src/invalidunroll.scala

+  def foo(int: Int, @unroll str: String = "1") = int.toString() + str
+
+  def fooInner(int: Int) = {
+    def inner(a: Int, @unroll str: String = "1") = str


full disclosure - local methods don't seem to get picked up by the plugin at all so they don't go through the validation phase which means that they are just silently ignored (on all Scala versions). I left this test here for visibility and discussion - should I remove the local checks and this definition?

Is there some way to make them get picked up by the compiler plugin? I would expect compiler plugins to have access to the entire AST of the file to walk through, which should let it find these problematic usages. If not that's fine too, but it seems like it should be doable

Yeah, I feel like it should be possible - I'll stare at it some more and report back with my findings

Managed to make it work with an additional traversal of the tree after we've matched a toplevel method.

lihaoyi · 2025-09-13T02:24:23Z

I think this looks fine @arainko, if you could update the PR description with your latest implementation details and (manual?) testing strategy I think we can merge this and close out the bounty

arainko · 2025-09-13T03:08:22Z

Done, thanks for taking a look!

lihaoyi · 2025-09-14T00:58:16Z

Thanks @arainko ! please email me your international bank transfer details and I will close out the bounty

arainko added 6 commits August 26, 2025 22:03

initial work, poking the plugin with a stick

30784d2

add Scala 3 check (not so pretty just yet)

b1644a4

adapt check from the compiler in the Scala 3 version of the plugin

8936b72

attempt Scala 2 check

0f90ba6

tune the Scala 2 check

5b7ca69

fix test compilation

5fb3d2a

remove abstractClassMethod and abstractTraitMethod tests, add `ne…

edaead4

…gative` module

arainko commented Sep 7, 2025

View reviewed changes

add LintInnerMethods check

2d36ed1

lihaoyi merged commit 48a4d96 into com-lihaoyi:main Sep 14, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Issue 12] `@unroll` on final or effectively final methods#15

[Issue 12] `@unroll` on final or effectively final methods#15
lihaoyi merged 8 commits intocom-lihaoyi:mainfrom
arainko:issue-12-unroll-on-finals

arainko commented Sep 2, 2025 •

edited

Loading

Uh oh!

lihaoyi commented Sep 2, 2025

Uh oh!

arainko commented Sep 2, 2025 •

edited

Loading

Uh oh!

lihaoyi commented Sep 2, 2025

Uh oh!

arainko Sep 7, 2025 •

edited

Loading

Uh oh!

lihaoyi Sep 8, 2025

Uh oh!

arainko Sep 8, 2025

Uh oh!

arainko Sep 12, 2025

Uh oh!

lihaoyi commented Sep 13, 2025

Uh oh!

arainko commented Sep 13, 2025

Uh oh!

Uh oh!

lihaoyi commented Sep 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

arainko commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihaoyi commented Sep 2, 2025

Uh oh!

arainko commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihaoyi commented Sep 2, 2025

Uh oh!

arainko Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihaoyi Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

arainko Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

arainko Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

lihaoyi commented Sep 13, 2025

Uh oh!

arainko commented Sep 13, 2025

Uh oh!

Uh oh!

lihaoyi commented Sep 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arainko commented Sep 2, 2025 •

edited

Loading

arainko commented Sep 2, 2025 •

edited

Loading

arainko Sep 7, 2025 •

edited

Loading