Move spec internals methods to be static method calls

[Vampire]

This removes relying on Groovy being able to call package-private methods in other-package subclasses, which does not work anymore with Groovy 5.

[Vampire]

• Add Groovy 5 support (#2196) #2213
• Move spec internals methods to be static method calls #2209 👈 (View in Graphite)
• Use snapshotId instead of separate tests #2207
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[leonard84]

What are the implications of using a mixin?

[Vampire]

The question is why the mock integration tests work differently.

But the intention of this is to prepare for the Groovy 5 PR.

By using the mixin (or a trait but there the method overload resolution is buggy unless we name the methods uniquely), we can bring in the spec internals methods at runtime.
This way the rewritten code can call them without the user seeing them in the code completion.

Up to now this was done by ensuring the methods are package-private, but Groovy 5 lost the ability to call them in that case.
Hence I searched for a replacement tactic and the mixin is what I came up with.

Except that it for some reason does not yet work in the mock integration tests and I have no idea yet why.

[leonard84]

Couldn't/Shouldn't we just generate static method calls to SpecialMethodCallTarget? I'm leery of using Groovy MOP if we don't have to.

[Vampire]

Actually, that was my first try.
But I did not make it too far.
Iirc it involved also making createStaticMock public and keeping the three create mock calls also in the specification and adding the specification or the specification context as first parameter to all methods in the remaining methods.

Afair the main problem was how to properly transform the instance method call to a static method call.
Currently (and with this PR) we just have to append Impl to the method name of the method call expression.
Or how to properly replace the old expression with the new one, especially in SpecialMethodCall where only the expression is present.

Another alternative would be to generate all those ...Impl methods into the compiled specification and either there have the implementation directly or from there make the static method call. But I don't think I like that idea. :-D

[Vampire]

Yeah, I think the main problem was how to replace the method call expression with a static method call expression.
I guess we would need to use a ExpressionReplacingVisitorSupport somehow.
Unfortunately it was added but never used anywhere, so I'm unsure how to properly replace the expression.
Do you know how to do it, then I'd give it another go with patching in static method calls.

[Vampire]

I have no idea which minor detail I did differently this time, but trying again it worked just fine to simply use a class expression as object expression for the method call expression to get a static call as result. 🤷‍♂️

So now no MOP, but static calls.

[codecov]

Codecov Report

❌ Patch coverage is 88.28829% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.93%. Comparing base (3493297) to head (098924f).

Files with missing lines	Patch %	Lines
...java/org/spockframework/runtime/SpecInternals.java	86.59%	12 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #2209   +/-   ##
=========================================
  Coverage     81.92%   81.93%           
+ Complexity     4732     4728    -4     
=========================================
  Files           463      463           
  Lines         14764    14768    +4     
  Branches       1869     1869           
=========================================
+ Hits          12096    12100    +4     
  Misses         1980     1980           
  Partials        688      688           

Files with missing lines	Coverage Δ
...java/org/spockframework/compiler/AstNodeCache.java	100.00% <100.00%> (ø)
...org/spockframework/compiler/DeepBlockRewriter.java	98.19% <100.00%> (+0.01%)	⬆️
...java/org/spockframework/compiler/SpecRewriter.java	93.76% <ø> (ø)
...org/spockframework/compiler/SpecialMethodCall.java	93.63% <100.00%> (+0.17%)	⬆️
.../org/spockframework/mock/EmptyOrDummyResponse.java	86.58% <100.00%> (ø)
...k-core/src/main/java/spock/lang/Specification.java	79.16% <100.00%> (+0.90%)	⬆️
...pock-core/src/main/java/spock/mock/MockingApi.java	93.65% <100.00%> (ø)
...java/org/spockframework/runtime/SpecInternals.java	86.59% <86.59%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AndreasTu]

LGTM.
Thank you!