[
https://issues.apache.org/jira/browse/GROOVY-12477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18130611#comment-18130611
]
ASF GitHub Bot commented on GROOVY-12477:
-----------------------------------------
codecov-commenter commented on PR #3006:
URL: https://github.com/apache/groovy/pull/3006#issuecomment-6351847788
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3006?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `77.77778%` with `2 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 72.4396%. Comparing base
([`6194927`](https://app.codecov.io/gh/apache/groovy/commit/61949271fc13446e8cf385d3e3e5968c6c72f92b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`91ee823`](https://app.codecov.io/gh/apache/groovy/commit/91ee823b05f2685dc0ca7e82971ddfcbc7506614?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3006?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...roovy/contracts/ast/ModifiesASTTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&filepath=subprojects%2Fgroovy-contracts%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fcontracts%2Fast%2FModifiesASTTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnRyYWN0cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L2NvbnRyYWN0cy9hc3QvTW9kaWZpZXNBU1RUcmFuc2Zvcm1hdGlvbi5qYXZh)
| 87.5000% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...s/ast/ModifiesEnsuresValidationTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&filepath=subprojects%2Fgroovy-contracts%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fcontracts%2Fast%2FModifiesEnsuresValidationTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnRyYWN0cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L2NvbnRyYWN0cy9hc3QvTW9kaWZpZXNFbnN1cmVzVmFsaWRhdGlvblRyYW5zZm9ybWF0aW9uLmphdmE=)
| 0.0000% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3006 +/- ##
==================================================
+ Coverage 72.4286% 72.4396% +0.0110%
- Complexity 40654 40666 +12
==================================================
Files 1643 1643
Lines 142811 142817 +6
Branches 26852 26854 +2
==================================================
+ Hits 103436 103456 +20
+ Misses 29827 29821 -6
+ Partials 9548 9540 -8
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3006?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...roovy/contracts/ast/ModifiesASTTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&filepath=subprojects%2Fgroovy-contracts%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fcontracts%2Fast%2FModifiesASTTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnRyYWN0cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L2NvbnRyYWN0cy9hc3QvTW9kaWZpZXNBU1RUcmFuc2Zvcm1hdGlvbi5qYXZh)
| `71.2121% <87.5000%> (+1.2121%)` | :arrow_up: |
|
[...s/ast/ModifiesEnsuresValidationTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3006?src=pr&el=tree&filepath=subprojects%2Fgroovy-contracts%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fcontracts%2Fast%2FModifiesEnsuresValidationTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnRyYWN0cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L2NvbnRyYWN0cy9hc3QvTW9kaWZpZXNFbnN1cmVzVmFsaWRhdGlvblRyYW5zZm9ybWF0aW9uLmphdmE=)
| `72.2222% <0.0000%> (ø)` | |
... and [11 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3006/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> @Modifies with an empty or no list isn't working correctly
> ----------------------------------------------------------
>
> Key: GROOVY-12477
> URL: https://issues.apache.org/jira/browse/GROOVY-12477
> Project: Groovy
> Issue Type: Bug
> Components: groovy-contracts, groovy-typecheckers
> Affects Versions: 6.0.0
> Reporter: Paul King
> Assignee: Paul King
> Priority: Minor
>
> h3. Problem
> There is no working way to declare that a method modifies nothing with
> {{@Modifies}}. The forms a user would reach for are either rejected or
> silently treated as if the method had no {{@Modifies}} at all.
> With {{@TypeChecked(extensions = 'groovy.typecheckers.ModifiesChecker')}} on
> a method whose body is {{balance -= n}}:
> ||Form||Result||
> |{{@Modifies(\{ this.other })}}|Error: {{'balance' is not declared in
> @Modifies}} (correct)|
> |{{@Modifies(\{ [] })}}|*Compiles; nothing is checked*|
> |{{@Modifies(\{})}}|Error: {{@Modifies closure must contain a field
> reference, parameter reference, or list of references}}|
> |{{@Modifies}}|*Compiles; nothing is checked*|
> {code:groovy}
> import groovy.contracts.Modifies
> import groovy.transform.TypeChecked
> @TypeChecked(extensions = 'groovy.typecheckers.ModifiesChecker')
> class Account {
> int balance
> @Modifies({ [] })
> void withdraw(int n) { balance -= n } // expected: error, actual:
> compiles
> }
> {code}
> h3. Cause
> * {{ModifiesASTTransformation}} stores the
> {{groovy.contracts.modifiesFields}} metadata only when the set is non-empty.
> An empty list therefore leaves no metadata. {{ModifiesChecker}} reads missing
> metadata as "no {{@Modifies}}, nothing to check".
> {{ModifiesEnsuresValidationTransformation}} returns early on an empty set
> too, so {{old.x}} in an {{@Ensures}} is not validated either.
> * An empty closure body is rejected because exactly one statement is required.
> * {{Modifies.value()}} has no default, so a bare {{@Modifies}} should be a
> missing-member error. The transform replaces {{value}} with {{Object.class}}
> unconditionally, which hides the missing member.
> The intent that "nothing" be expressible is already in the docs and the
> checker. The type checkers guide says {{@Pure}}, {{@SideEffectFree}} and
> {{@Contract(pure = true)}} each imply {{@Modifies({})}}, and
> {{ModifiesChecker}} substitutes an empty frame for them. A user cannot write
> that form themselves.
> h3. Proposed fix
> * {{@Modifies({})}} and {{@Modifies(\{ [] })}} both declare an empty frame:
> the method modifies no fields or parameters. The metadata is always stored,
> even when empty, so an empty frame is distinguishable from no annotation.
> * A bare {{@Modifies}} is a compile error. A frame condition is a claim the
> checker enforces, so "modifies nothing" should be explicit rather than
> inferred from a marker that could equally be read as "modifies something
> unspecified".
> * With an empty frame, {{@Ensures}} may not reference {{old.<field>}},
> consistent with the existing rule that {{old}} may only reference fields
> declared in {{@Modifies}}.
> * The contracts user guide states the empty-frame form.
> Both modules are {{@Incubating}}, so the tightening is low cost. The only
> existing code that changes behaviour is code using {{@Modifies(\{ [] })}} or
> a bare {{@Modifies}} and relying on it being unchecked.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
