bug(#744): optimized redundant-object lint#779
bug(#744): optimized redundant-object lint#779h1alexbel wants to merge 3 commits intoobjectionary:masterfrom
redundant-object lint#779Conversation
WalkthroughRefactors redundant-object lint XSL to compute eligible objects, group references, and emit a single warning-style defect element per object based on refs count and context rules. Updates benchmark to stop excluding “redundant-object” from scansXmir, leaving only “duplicate-names-in-diff-context” excluded. Changes
Sequence Diagram(s)sequenceDiagram
participant XMIR as XMIR doc
participant XSL as redundant-object.xsl
participant Lints as Lints Output
XMIR->>XSL: Apply lint template
rect rgba(200,230,255,0.3)
note over XSL: New selection flow
XSL->>XSL: Compute eligible objects
XSL->>XSL: Group refs by base-name pattern (ref-groups)
loop For each eligible (excluding top-generated-id)
XSL->>XSL: refs = count from ref-groups
alt refs ≤ 1 and not dataized
XSL-->>Lints: Emit <defect line=... severity="warning"[ context? ]/>
else No defect
XSL-->>Lints: Skip
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/resources/org/eolang/lints/misc/redundant-object.xsl (1)
22-22: Simplify condition (redundant@namecheck)
$eligiblealready guarantees@name; drop it from the guard.- <xsl:if test="$refs <= 1 and not(@name and o[1]/@base = 'Φ.org.eolang.dataized')"> + <xsl:if test="$refs <= 1 and not(o[1]/@base = 'Φ.org.eolang.dataized')">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/resources/org/eolang/lints/misc/redundant-object.xsl(1 hunks)src/test/java/benchmarks/SourceBench.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: reserved
- GitHub Check: qulice
- GitHub Check: mvn (windows-2022, 21)
- GitHub Check: mvn (macos-15, 11)
- GitHub Check: ort
- GitHub Check: mvn (windows-2022, 11)
- GitHub Check: mvn (macos-15, 21)
- GitHub Check: mvn (ubuntu-24.04, 21)
- GitHub Check: mvn (ubuntu-24.04, 11)
- GitHub Check: deep
🔇 Additional comments (2)
src/test/java/benchmarks/SourceBench.java (1)
51-52: LGTM: enabling redundant-object in scansRemoving it from exclusions aligns with the lint changes and reported perf gains.
src/main/resources/org/eolang/lints/misc/redundant-object.xsl (1)
23-30: Confirmed attribute-based severity support All XSL templates emit<defect>with aseverityattribute and Java consumers parse@severity; nested<severity/>elements are not used.
| <xsl:variable name="ref-groups" as="element(group)*"> | ||
| <xsl:for-each-group select="//o[matches(@base, '^ξ(?:\.ρ)*\.')]" group-by="replace(@base, '^ξ(?:\.ρ)*\.([^.\s]+).*', '$1')"> | ||
| <group name="{current-grouping-key()}" count="{count(current-group())}"/> | ||
| </xsl:for-each-group> | ||
| </xsl:variable> |
There was a problem hiding this comment.
Refs are counted by name globally; collisions across scopes → incorrect results
Grouping by only the name merges references from different lexical scopes (e.g., multiple siblings each having o @name="tmp"). This can suppress true defects or create false negatives/positives. Group by nearest scope + name, and use that for $refs.
Apply this diff:
- <xsl:variable name="ref-groups" as="element(group)*">
- <xsl:for-each-group select="//o[matches(@base, '^ξ(?:\.ρ)*\.')]" group-by="replace(@base, '^ξ(?:\.ρ)*\.([^.\s]+).*', '$1')">
- <group name="{current-grouping-key()}" count="{count(current-group())}"/>
- </xsl:for-each-group>
- </xsl:variable>
+ <xsl:variable name="ref-groups" as="element(group)*">
+ <!-- Group by lexical scope (nearest ancestor o) + local name to avoid cross-scope collisions -->
+ <xsl:for-each-group
+ select="//o[matches(@base, '^ξ(?:\.ρ)*\.')]"
+ group-by="concat(generate-id(ancestor::o[1]), '::', replace(@base, '^ξ(?:\.ρ)*\.([^.\s]+).*', '$1'))">
+ <group
+ scope="{generate-id(current-group()[1]/ancestor::o[1])}"
+ name="{replace(current-group()[1]/@base, '^ξ(?:\.ρ)*\.([^.\s]+).*', '$1')}"
+ count="{count(current-group())}"/>
+ </xsl:for-each-group>
+ </xsl:variable>
@@
- <xsl:variable name="refs" select="number(($ref-groups[@name = current()/@name]/@count, 0)[1])"/>
+ <xsl:variable name="refs"
+ select="number(($ref-groups[@name = current()/@name and @scope = generate-id(ancestor::o[1])]/@count, 0)[1])"/>Also applies to: 21-21
🤖 Prompt for AI Agents
In src/main/resources/org/eolang/lints/misc/redundant-object.xsl around lines 13
to 17 (also apply same change at line 21), the current grouping key uses only
the object name which causes collisions across different lexical scopes; change
the group-by to include the nearest scope identifier plus the name (e.g.,
compute or capture the nearest enclosing scope node or its unique id and combine
it with replace(@base, '^ξ(?:\.ρ)*\.([^.\s]+).*', '$1') into the grouping key)
and then use that combined scope+name key for $refs so references are counted
per-scope rather than globally.
1 participant
In this PR I've optimized
redundant-objectlint fromO(n^2)toO(n + G)execution time, wheren- the number ofobjects, andG- one-time grouping pass.Evaluation
To evaluate results, I used the following script
Before:
After:
see #744
Summary by CodeRabbit
Bug Fixes
Tests