Skip to content

refactor: extract shared getBinaryLogicalOperator helper - #597

Merged
askpt merged 2 commits into
mainfrom
repo-assist/refactor-shared-binary-logical-operator-20260902-0d6cc91f280e6ca5
Sep 2, 2026
Merged

refactor: extract shared getBinaryLogicalOperator helper#597
askpt merged 2 commits into
mainfrom
repo-assist/refactor-shared-binary-logical-operator-20260902-0d6cc91f280e6ca5

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

What

Extracts the byte-for-byte identical getBinaryOperator private method that was duplicated across csharpAnalyzer.ts, goAnalyzer.ts, javaAnalyzer.ts, and rustAnalyzer.ts into a single shared getBinaryLogicalOperator export in complexityHelpers.ts.

Why

All four analyzers had the exact same logic (read child(1) of a binary_expression node, check if its type is && or ||). This is the same class of duplication the repo already eliminated for isOutermostInSameOperatorChain in a previous pass — this helper was simply missed. Consolidating it in one place reduces maintenance surface and keeps the four analyzers consistent by construction.

Trade-offs

Pure refactor, no behavior change. One previously-covered defensive if (!operatorNode) branch became structurally unreachable from all four call sites combined (tree-sitter always emits an operator token for a valid binary_expression), so it now carries a /* c8 ignore next */ annotation consistent with the existing pattern used for similar unreachable defensive branches elsewhere in this file.

Test Status

  • npm run compile: ✅ clean
  • npm run lint: ✅ clean
  • npm run test:unit: ✅ 241/241 passing, coverage 98.76% stmts / 95.67% branches / 99.08% funcs / 98.76% lines (all above the configured .c8rc.json thresholds of 95/88/97/95)
  • npm run test:vscode (integration): not run — blocked by sandboxed network (getaddrinfo EAI_AGAIN update.code.visualstudio.com), same known limitation as all previous Repo Assist runs in this environment.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by 🌈 Repo Assist, see workflow run. Learn more.
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@42c2ab5b4e4c9273534c39259b2e0df7f20f07e9

Extracts the byte-for-byte identical private getBinaryOperator method
that was duplicated across csharpAnalyzer, goAnalyzer, javaAnalyzer,
and rustAnalyzer into a single shared getBinaryLogicalOperator export
in complexityHelpers.ts (alongside isOutermostInSameOperatorChain,
which already lives there for the same reason).

Pure refactor, no behavior change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@askpt askpt changed the title [repo-assist] refactor: extract shared getBinaryLogicalOperator helper refactor: extract shared getBinaryLogicalOperator helper Sep 2, 2026
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.19%. Comparing base (3296a56) to head (a4f05de).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
+ Coverage   98.11%   98.19%   +0.08%     
==========================================
  Files          12       12              
  Lines        3931     3885      -46     
  Branches      451      440      -11     
==========================================
- Hits         3857     3815      -42     
+ Misses         74       70       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@askpt
askpt marked this pull request as ready for review September 2, 2026 07:48
@askpt
askpt self-requested a review as a code owner September 2, 2026 07:48
Copilot AI balanced review requested due to automatic review settings September 2, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The C# analyzer now has misleading stale JSDoc attached to getComplexityReason.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Centralizes logical binary-operator extraction without changing analyzer behavior.

Changes:

  • Adds shared getBinaryLogicalOperator.
  • Updates four analyzers and removes duplicate methods.
  • Leaves stale C# JSDoc requiring cleanup.
File summaries
File Description
complexityHelpers.ts Adds shared helper.
csharpAnalyzer.ts Uses helper; removes duplicate.
goAnalyzer.ts Uses helper; removes duplicate.
javaAnalyzer.ts Uses helper; removes duplicate.
rustAnalyzer.ts Uses helper; removes duplicate.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/metricsAnalyzer/languages/csharpAnalyzer.ts Outdated
@askpt

askpt commented Sep 2, 2026

Copy link
Copy Markdown
Owner

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

Co-authored-by: askpt <2493377+askpt@users.noreply.github.com>

Copilot AI commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply t...

Addressed in commit a4f05de by removing the stale JSDoc lines so getComplexityReason now has a single correct doc block.

@askpt
askpt merged commit ebe1a46 into main Sep 2, 2026
8 checks passed
@askpt
askpt deleted the repo-assist/refactor-shared-binary-logical-operator-20260902-0d6cc91f280e6ca5 branch September 2, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants