You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "eric-milles (via GitHub)" <gi...@apache.org> on 2026/05/31 16:39:08 UTC

[PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

eric-milles opened a new pull request, #2580:
URL: https://github.com/apache/groovy/pull/2580

   Should `map['key']` work like `map.key` or `map.get('key')`?  In older Groovy versions, `getAt(Map,Object)` was considered closer than `getAt(Object,String)`.  Adding `getAt(Map,String)` and `putAt(Map,String,V)` restores alignment between `getAt` and `get` at the expense of the public property handling for the subscript syntax.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4587478098

   ### JMH summary — indy (commit `84922ba`)
   
   Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
   `1.00` = in line with history. Per-benchmark ratio, geomean within group.
   Time-per-op units inverted so direction is consistent.
   
   | Group  | Speedup | n |
   |--------|---------|---|
   | bench | 0.974 × | 26 |
   | core | 0.993 × | 77 |
   | grails | 1.039 × | 80 |
   
   <sub>Baseline: <code>dev/bench/jmh/&lt;part&gt;/indy/data.js</code> on gh-pages, trailing 90 days. <a href="https://apache.github.io/groovy/dev/bench/jmh/summary.html">Daily dashboard</a> · <a href="https://apache.github.io/groovy/dev/bench/jmh/">Per-suite raw data</a></sub>
   
   <!-- jmh-summary:indy -->


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert merged PR #2580:
URL: https://github.com/apache/groovy/pull/2580


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4603356488

   I think this is an improvement, I will try to do an assessment of all the cases tomorrow.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4587461623

   ### JMH summary — classic (commit `84922ba`)
   
   Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
   `1.00` = in line with history. Per-benchmark ratio, geomean within group.
   Time-per-op units inverted so direction is consistent.
   
   | Group  | Speedup | n |
   |--------|---------|---|
   | bench | 0.997 × | 26 |
   | core | 1.141 × | 77 |
   | grails | 0.977 × | 80 |
   
   <sub>Baseline: <code>dev/bench/jmh/&lt;part&gt;/classic/data.js</code> on gh-pages, trailing 90 days. <a href="https://apache.github.io/groovy/dev/bench/jmh/summary.html">Daily dashboard</a> · <a href="https://apache.github.io/groovy/dev/bench/jmh/">Per-suite raw data</a></sub>
   
   <!-- jmh-summary:classic -->


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4587478374

   ## ✅ All tests passed ✅
   
   🏷️ Commit: dc01f8c48173b26b502909858699d13f5cba2f33
   ▶️ Tests:  100169 executed
   ⚪️ Checks: 30/30 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4665157169

   The above table was wrong. Here is the revised one:
   
   For reference, here's the full `m.k` vs `m['k']` vs `m.get('k')` matrix on 6.0 after this change, across dynamic and `@CompileStatic` access. Every cell is now pinned by the live tables in `_working-with-collections.adoc` / `WorkingWithCollectionsTest.groovy`.
   
   **Plain map** — `[foo:'bar']` / `[class:'C']` / `[:]`
   
   | key                                 | `m.k`                        | `m.k` (`@CompileStatic`) | `m['k']` | `m.get('k')` |
   | ----------------------------------- | ---------------------------- | ------------------------ | -------- | ------------ |
   | ordinary (`'foo'`)                  | `'bar'`                      | `'bar'`                  | `'bar'`  | `'bar'`      |
   | meta-name present (`'class'`→`'C'`) | `'C'` (entry, not the Class) | `'C'`                    | `'C'`    | `'C'`        |
   | meta-name absent (`'class'`)        | `null`                       | `null`                   | `null`   | `null`       |
   
   **Map subclass** — `class C extends HashMap { def foo = 1; def bar = 2 }`, then `put('foo',11)`, `put('baz',33)`
   
   | key                             | `m.k` | `m.k` (`@CompileStatic`) | `m['k']` | `m.get('k')` |
   | ------------------------------- | ----- | ------------------------ | -------- | ------------ |
   | `'foo'` (property=1, entry=11)  | `1`   | `1`                      | `11`     | `11`         |
   | `'bar'` (property=2, no entry)  | `2`   | `2`                      | `null`   | `null`       |
   | `'baz'` (no property, entry=33) | `33`  | `33`                     | `33`     | `33`         |
   
   Takeaways:
   
   1. **`m['k']` ≡ `m.get('k')` in every row and in both modes** — that's the invariant this PR restores (subscript was previously diverging under static resolution).
   2. **`m.k` and `m['k']` agree on a plain map** but legitimately differ on a subclass: `m.k` reads the declared property when one exists (falling back to the entry), while `m['k']`/`get` always read the entry. Dynamic and `@CompileStatic` columns now match throughout, so the only axis that matters is property-vs-entry, not dynamic-vs-static.
   3. Practical guidance: use `['k']`/`get` whenever you mean "the entry"; reserve `m.k` for property semantics.
   
   ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4587429972

   ## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2580?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 68.1816%. Comparing base ([`a3b47f2`](https://app.codecov.io/gh/apache/groovy/commit/a3b47f2b70c0236067ee20033b54b8a4c5c0c414?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`dc01f8c`](https://app.codecov.io/gh/apache/groovy/commit/dc01f8c48173b26b502909858699d13f5cba2f33?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/groovy/pull/2580/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2580?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2580        +/-   ##
   ==================================================
   + Coverage     68.1808%   68.1816%   +0.0008%     
     Complexity      33164      33164                
   ==================================================
     Files            1511       1511                
     Lines          126232     126235         +3     
     Branches        22897      22897                
   ==================================================
   + Hits            86066      86069         +3     
   + Misses          32527      32526         -1     
   - Partials         7639       7640         +1     
   ```
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2580?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [.../codehaus/groovy/runtime/DefaultGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/2580?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FDefaultGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvRGVmYXVsdEdyb292eU1ldGhvZHMuamF2YQ==) | `75.1044% <100.0000%> (+0.0208%)` | :arrow_up: |
   
   ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2580/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>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4587453544

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2580) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2580&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2580&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2580&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2580&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2580&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2580)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4664134906

   > The key reframing: under the **dynamic** runtime, `m['k']` already means `m.get('k')` for every key — `[:].class`, `[:].properties`, `[:].metaClass` all return `null` (the absent entry), not the bean. The surprising behavior was confined to **static** resolution, where subscript was binding to the property-access overload. So this PR isn't trading one consistency for another so much as **realigning STC subscript with the long-standing dynamic semantics** (and with `get`/`put`).
   >
   > Verified values (dynamic against the runtime; static column = intended 6.0 / the STC test assertions in this PR):
   >
   > **Plain map** — `[foo:'bar']` / `[class:'C']` / `[:]`
   >
   > | key | `m.k` dynamic | `m.k` `@CompileStatic` | `m['k']` (6.0) | `m.get('k')` |
   > |---|---|---|---|---|
   > | ordinary (`'foo'`) | `'bar'` | `'bar'` | `'bar'` | `'bar'` |
   > | meta-name present (`'class'`→`'C'`) | `'C'` (entry, not the Class) | `'C'` | `'C'` | `'C'` |
   > | meta-name absent (`'class'`) | `null` | `null` | `null` | `null` |
   >
   > **Map subclass** — `class C extends HashMap { def foo = 1; def bar = 2 }`, then `put('foo',11)`, `put('baz',33)`
   >
   > | key | `m.k` dynamic | `m.k` `@CompileStatic` | `m['k']` (6.0) | `m.get('k')` |
   > |---|---|---|---|---|
   > | declared prop **and** entry (`'foo'`: prop=1, entry=11) | **`11`** (entry wins) | **`1`** (property wins) | `11` | `11` |
   > | declared prop, **no** entry (`'bar'`: prop=2) | **`null`** (property ignored) | **`2`** (property) | `null` | `null` |
   > | entry, **no** declared prop (`'baz'`: entry=33) | `33` | `33` | `33` | `33` |
   >
   > Two takeaways:
   >
   > 1. **After this PR, `m['k']` ≡ `m.get('k')` in every row and both modes** — one clean column, no asterisks. That's the win worth stating explicitly as the invariant.
   > 2. The genuinely treacherous cell is **`m.k` on a Map subclass**, which disagrees dynamic-vs-static (entry wins dynamically, declared property wins under STC). This PR doesn't touch dot access, so that divergence remains — subscript just sidesteps it now. Might be worth a follow-up issue, or at least a doc note steering entry access to `[]`/`get`.
   >
   > Suggestion for locking it in: pin the invariant as a paired test that runs the same asserts under dynamic **and** `@CompileStatic`, so the static/dynamic gap that caused this can't silently reopen via overload-resolution drift. Happy to put that together if helpful.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-12024: add `getAt(Map,String)` and `putAt(Map,String,V)` [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2580:
URL: https://github.com/apache/groovy/pull/2580#issuecomment-4665165449

   To keep this up -to-date, I added a "live table" of these results in:
   src/spec/doc/_working-with-collections.adoc


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org