[
https://issues.apache.org/jira/browse/GROOVY-12729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12729:
-------------------------------
Description:
Probed on master (715553465f). Each loop runs 3 times. The results are the same
in a script and in a class method.
h3. 1. An empty closure or a qualified class literal turns off the check
||Annotation on the loop||Result||
|{{@Invariant(\{\})}}|runs; no check is made, and nothing reports that|
|{{@Invariant(java.lang.Object)}}|runs; no check is made|
|{{@Decreases(\{\})}}|runs; no check is made, even on a loop that does not
decrease|
|{{@Decreases(java.lang.Object)}}|runs; no check is made|
Controls: {{@Invariant(\{ i < 2 \})}} throws {{LoopInvariantViolation}}, and
{{@Decreases(\{ n \})}} on a loop that does not decrease throws
{{LoopVariantViolation: loop variant did not decrease: was 3, now 3}}.
{{LoopInvariantASTTransformation}} and {{LoopVariantASTTransformation}} return
without reporting anything in either case: when the value is not a closure, or
when the closure has no expression.
Class-level contracts already reject an empty closure. {{@Invariant(\{\})}} on
a class fails to compile with:
{noformat}
[groovy-contracts] Annotation does not contain any expressions (e.g. use
'@Requires({ argument1 })').
{noformat}
The loop forms should behave the same.
h3. 2. {{@Decreases(\{ [] \})}} can never pass
It throws {{LoopVariantViolation: loop variant did not decrease: was [], now
[]}} on every loop that runs more than once, including one that does decrease
{{n}}. An empty tuple never decreases, so a compile error would be more helpful.
{{@Invariant(\{ [] \})}} also always fails, but that is correct, since an empty
list is false in Groovy, as with {{\{ false \}}}. Class-level {{@Invariant(\{
[] \})}} behaves the same way.
h3. 3. Simple class names in loop annotations are never resolved
||Annotation on the loop||Result||
|{{@Invariant(Object)}}, {{@Decreases(Object)}}|{{Only classes, closures, and
method references can be used for attribute 'value' in
@groovy.contracts.Invariant}} (or {{Decreases}})|
|{{@Invariant(java.lang.Object)}}, {{@Decreases(java.lang.Object)}}|compiles,
and is then silently ignored (see 1)|
|{{@Invariant(Object.class)}}, {{@Decreases(Object.class)}}|{{unable to find
class 'Object.class' for annotation attribute constant}}|
So the simple name is rejected only by accident. An AST probe at CONVERSION,
SEMANTIC_ANALYSIS and CANONICALIZATION shows the {{@Decreases(Object)}} member
staying a {{VariableExpression(Object)}} throughout. A qualified name becomes a
{{ClassExpression}} during SEMANTIC_ANALYSIS, and on a method annotation,
{{@Requires(Object)}} becomes a {{ClassExpression}} as expected. This affects
any annotation on a loop, not just contracts.
A list value ({{@Invariant(['i < 2'])}}, {{@Decreases([1])}}) is correctly
rejected by the generic annotation check.
h3. Suggested fix
* Loop {{@Invariant}} and {{@Decreases}}: report an error for a value that is
not a closure, and for an empty closure, as class-level contracts do. Also
report one for {{@Decreases(\{ [] \})}}.
* Resolve class names in loop-statement annotations the same way as in
declaration annotations (possibly a separate issue).
Related: GROOVY-12128 (where loop contract checks fire), GROOVY-12723
(@Modifies), and the issue raised alongside this one (class literals on
@Requires, @Ensures and @Invariant).
was:
Probed on master (715553465f). Each loop runs 3 times. "Decreasing" is {{while
(n > 0) \{ n-- \}}}, and "not decreasing" is {{while (i < 3) \{ i++ \}}}.
||Annotation||Decreasing loop||Loop that does not decrease||
|{{@Decreases(\{ n \})}}, {{@Decreases(\{ [n] \})}}
(controls)|OK|{{LoopVariantViolation: loop variant did not decrease: was 3, now
3}}|
|{{@Decreases(\{\})}}|OK|*OK*: no check is made, and nothing reports that|
|{{@Decreases(\{ [] \})}}|*{{LoopVariantViolation: loop variant did not
decrease: was [], now []}}*|same|
|{{@Decreases(java.lang.Object)}}|OK|*OK*: no check is made|
|{{@Decreases(Object)}}|compile error: {{Only classes, closures, and method
references can be used for attribute 'value' in @groovy.contracts.Decreases}}| |
|{{@Decreases([1])}}|same compile error| |
|{{@Decreases(Object.class)}}|compile error: {{unable to find class
'Object.class' for annotation attribute constant}}| |
The results are the same in a script and in a class method.
h3. 1. Values that disable the check
* {{LoopVariantASTTransformation}} returns without doing anything when the
value is not a closure. It does the same when the closure has no expression. So
{{@Decreases(\{\})}} and a qualified class literal compile to an unchecked loop.
* Unlike {{@Modifies(\{\})}}, which declares an empty frame, an empty
{{@Decreases}} has no meaning.
h3. 2. {{\{ [] \}}} can never pass
An empty tuple never decreases, so any loop that runs more than once fails.
That is correct as far as it goes, but the result is almost certainly not what
was meant.
h3. 3. Simple class names in loop annotations are never resolved
{{@Decreases(Object)}} is rejected only by accident. An AST probe at
CONVERSION, SEMANTIC_ANALYSIS and CANONICALIZATION shows the member value
staying a {{VariableExpression(Object)}} throughout, so the generic annotation
check rejects it. A qualified {{java.lang.Object}} becomes a
{{ClassExpression}} during SEMANTIC_ANALYSIS. On a method annotation,
{{@Requires(Object)}} becomes a {{ClassExpression}} as expected.
{{Object.class}} gives the misleading "unable to find class 'Object.class'".
This affects any annotation placed on a loop, not just {{@Decreases}}.
h3. Suggested fix
* {{@Decreases}}: report an error for a value that is not a closure, for an
empty closure, and for an empty list, in the style of the {{@Modifies}} errors
(GROOVY-12723).
* Resolve class names in loop-statement annotations the same way as in
declaration annotations. This could be a separate issue.
Related: GROOVY-12128 (where loop contract checks fire), GROOVY-12723
(@Modifies), and the issue raised alongside this one (class literals on
@Requires, @Ensures and @Invariant).
> groovy-contracts: @Decreases is silently disabled by {} or by a qualified
> class literal, rejects { [] } on every loop, and rejects a simple class name
> only because it is never resolved
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12729
> URL: https://issues.apache.org/jira/browse/GROOVY-12729
> Project: Groovy
> Issue Type: Bug
> Components: groovy-contracts
> Reporter: Paul King
> Priority: Major
>
> Probed on master (715553465f). Each loop runs 3 times. The results are the
> same in a script and in a class method.
> h3. 1. An empty closure or a qualified class literal turns off the check
> ||Annotation on the loop||Result||
> |{{@Invariant(\{\})}}|runs; no check is made, and nothing reports that|
> |{{@Invariant(java.lang.Object)}}|runs; no check is made|
> |{{@Decreases(\{\})}}|runs; no check is made, even on a loop that does not
> decrease|
> |{{@Decreases(java.lang.Object)}}|runs; no check is made|
> Controls: {{@Invariant(\{ i < 2 \})}} throws {{LoopInvariantViolation}}, and
> {{@Decreases(\{ n \})}} on a loop that does not decrease throws
> {{LoopVariantViolation: loop variant did not decrease: was 3, now 3}}.
> {{LoopInvariantASTTransformation}} and {{LoopVariantASTTransformation}}
> return without reporting anything in either case: when the value is not a
> closure, or when the closure has no expression.
> Class-level contracts already reject an empty closure. {{@Invariant(\{\})}}
> on a class fails to compile with:
> {noformat}
> [groovy-contracts] Annotation does not contain any expressions (e.g. use
> '@Requires({ argument1 })').
> {noformat}
> The loop forms should behave the same.
> h3. 2. {{@Decreases(\{ [] \})}} can never pass
> It throws {{LoopVariantViolation: loop variant did not decrease: was [], now
> []}} on every loop that runs more than once, including one that does decrease
> {{n}}. An empty tuple never decreases, so a compile error would be more
> helpful.
> {{@Invariant(\{ [] \})}} also always fails, but that is correct, since an
> empty list is false in Groovy, as with {{\{ false \}}}. Class-level
> {{@Invariant(\{ [] \})}} behaves the same way.
> h3. 3. Simple class names in loop annotations are never resolved
> ||Annotation on the loop||Result||
> |{{@Invariant(Object)}}, {{@Decreases(Object)}}|{{Only classes, closures, and
> method references can be used for attribute 'value' in
> @groovy.contracts.Invariant}} (or {{Decreases}})|
> |{{@Invariant(java.lang.Object)}}, {{@Decreases(java.lang.Object)}}|compiles,
> and is then silently ignored (see 1)|
> |{{@Invariant(Object.class)}}, {{@Decreases(Object.class)}}|{{unable to find
> class 'Object.class' for annotation attribute constant}}|
> So the simple name is rejected only by accident. An AST probe at CONVERSION,
> SEMANTIC_ANALYSIS and CANONICALIZATION shows the {{@Decreases(Object)}}
> member staying a {{VariableExpression(Object)}} throughout. A qualified name
> becomes a {{ClassExpression}} during SEMANTIC_ANALYSIS, and on a method
> annotation, {{@Requires(Object)}} becomes a {{ClassExpression}} as expected.
> This affects any annotation on a loop, not just contracts.
> A list value ({{@Invariant(['i < 2'])}}, {{@Decreases([1])}}) is correctly
> rejected by the generic annotation check.
> h3. Suggested fix
> * Loop {{@Invariant}} and {{@Decreases}}: report an error for a value that is
> not a closure, and for an empty closure, as class-level contracts do. Also
> report one for {{@Decreases(\{ [] \})}}.
> * Resolve class names in loop-statement annotations the same way as in
> declaration annotations (possibly a separate issue).
> Related: GROOVY-12128 (where loop contract checks fire), GROOVY-12723
> (@Modifies), and the issue raised alongside this one (class literals on
> @Requires, @Ensures and @Invariant).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
