feat: migrate unsafe_type_assertion from regex to AST detection - #24
Merged
Conversation
- Replace regex-based detection with TypeScript Compiler API - Comments and strings containing are no longer reported - Nested and multiline assertions handled syntactically - Pattern ID unchanged — existing severityOverrides continue to work - 10 new regression tests covering detection, false positives, suppression
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesUnsafe assertion detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SourceFile
participant analyzeFile
participant findUnsafeAssertions
participant TypeScriptAST
participant SuppressionChecks
SourceFile->>analyzeFile: provide source text
analyzeFile->>findUnsafeAssertions: inspect assertions
findUnsafeAssertions->>TypeScriptAST: locate assertion target types
TypeScriptAST-->>findUnsafeAssertions: return assertion nodes and positions
findUnsafeAssertions->>SuppressionChecks: evaluate suppression directives
SuppressionChecks-->>analyzeFile: return applicable findings
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
codeant-ai
Bot
added
the
size:M
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+245
to
+247
| // Use the position of the type node (same line as `as` keyword) for correct line reporting | ||
| const { line: asLineIdx, character: asCharIdx } = sourceFile.getLineAndCharacterOfPosition(node.type.getStart(sourceFile)); | ||
| const asLine = asLineIdx + 1; |
There was a problem hiding this comment.
Suggestion: The finding location is now taken from node.type, while suppression is checked earlier using the outer assertion's sourceLine. For a multiline assertion such as value as\\n any, a suppression directive on the reported any line is evaluated against line 1 and is missed, so the new multiline detection cannot be suppressed at the location it reports. Check suppression against the reported line as well, or keep the finding location and suppression basis consistent. [logic error]
Severity Level: Major ⚠️
- ⚠️ Multiline assertion suppressions are ineffective.
- ⚠️ Reported locations disagree with suppression checks.
- ❌ Explicit lint directives cannot silence affected findings.Steps of Reproduction ✅
1. Call `findUnsafeAssertions()` with the multiline assertion form exercised by
`tests/ai-slop-detector.behavior.test.ts:263-271`: `const x = value as` followed by
`any;`.
2. The visitor enters the outer `ts.AsExpression` and checks suppression against its start
line at `ai-slop-detector.ts:227`; for this form, the outer expression starts on line 1.
3. The finding location is then calculated from `node.type.getStart(sourceFile)` at
`ai-slop-detector.ts:246-247`, so the reported finding is line 2, where the `any` type
node begins.
4. Put `// eslint-disable-line` on the reported line (`any; // eslint-disable-line`).
`lineIsSuppressed()` at `ai-slop-detector.ts:183-199` would recognize that directive for
line 2, but the visitor already tested line 1 and proceeds to emit the finding. Thus
suppression at the reported location fails for multiline assertions.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 245:247
**Comment:**
*Logic Error: The finding location is now taken from `node.type`, while suppression is checked earlier using the outer assertion's `sourceLine`. For a multiline assertion such as `value as\\n any`, a suppression directive on the reported `any` line is evaluated against line 1 and is missed, so the new multiline detection cannot be suppressed at the location it reports. Check suppression against the reported line as well, or keep the finding location and suppression basis consistent.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| pattern.id === 'unsafe_object_assertion' || | ||
| pattern.id === 'unsafe_array_assertion') { | ||
| pattern.id === 'unsafe_array_assertion' || | ||
| pattern.id === 'unsafe_type_assertion') { |
There was a problem hiding this comment.
Suggestion: The regex path now skips unsafe_type_assertion for every target extension, but the AST pass below runs only for .ts and .tsx. As a result, .js and .jsx files no longer produce this finding even though they remain supported scan targets and previously went through the regex detector. Either run the AST detection for those extensions with the appropriate script kind or retain equivalent detection for them. [api mismatch]
Severity Level: Critical 🚨
- ❌ JavaScript assertions are no longer reported.
- ❌ JSX assertion coverage is also lost.
- ⚠️ Supported target extensions behave inconsistently.Steps of Reproduction ✅
1. Run the detector against a project containing `.js` or `.jsx` files;
`AISlopDetector.targetExtensions` explicitly includes these extensions at
`ai-slop-detector.ts:296`.
2. A target file containing `value as any` reaches the generic pattern loop in
`analyzeFile()` at `ai-slop-detector.ts:982-1000`.
3. The new condition at `ai-slop-detector.ts:998` skips every pattern whose ID is
`unsafe_type_assertion`, so the regex path cannot report the assertion.
4. The AST pass is guarded by `filePath.endsWith('.ts') || filePath.endsWith('.tsx')` at
`ai-slop-detector.ts:1173-1174`, so it never calls `findUnsafeAssertions()` for `.js` or
`.jsx` files. The result is no `unsafe_type_assertion` finding for those supported target
extensions.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 998:998
**Comment:**
*Api Mismatch: The regex path now skips `unsafe_type_assertion` for every target extension, but the AST pass below runs only for `.ts` and `.tsx`. As a result, `.js` and `.jsx` files no longer produce this finding even though they remain supported scan targets and previously went through the regex detector. Either run the AST detection for those extensions with the appropriate script kind or retain equivalent detection for them.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Owner
Author
|
Tracked the column/line inconsistency as issue #25 for follow-up validation. |
When the outer assertion type is also , both unsafe_double_type_assertion and unsafe_type_assertion are now emitted independently. Disabling one rule does not suppress the other.
Owner
Author
Fix appliedRegression: Fix: Added an inner check inside the double-assertion branch so Verification
32/32 tests pass. Build succeeds. No unrelated changes. Please re-review. |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/ai-slop-detector.behavior.test.ts`:
- Around line 282-290: Update findUnsafeAssertions to inspect inline suppression
directives on currentLine before its early return for first-line input. Ensure
`// eslint-disable-line` suppresses findings such as `const x = value as any; //
eslint-disable-line` on line 1, while preserving the existing preceding-line
suppression behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: de7bf31a-5414-42c2-95a0-2b6df67f44f3
📒 Files selected for processing (4)
CHANGELOG.mdai-slop-detector.tspackage.jsontests/ai-slop-detector.behavior.test.ts
Comment on lines
+282
to
+290
| test('findUnsafeAssertions does not report as any when preceded by @ts-expect-error', () => { | ||
| const code = '// @ts-expect-error\nconst x = value as any;'; | ||
| assert.equal(findUnsafeAssertions(code, 'x.ts').length, 0); | ||
| }); | ||
|
|
||
| test('findUnsafeAssertions does not report as any when preceded by eslint-disable-next-line', () => { | ||
| const code = '// eslint-disable-next-line @typescript-eslint/no-unused-vars\nconst x = value as any;'; | ||
| assert.equal(findUnsafeAssertions(code, 'x.ts').length, 0); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle first-line inline suppressions.
These tests only cover preceding-line directives. Line 184 returns before inspecting currentLine, so const x = value as any; // eslint-disable-line on line 1 still reports a finding.
Proposed fix
const lineIsSuppressed = (line: number): boolean => {
- if (line <= 1) return false;
- const prevLine = lines[line - 2];
+ if (line < 1 || line > lines.length) return false;
+ const prevLine = line > 1 ? lines[line - 2] : '';
const currentLine = lines[line - 1];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/ai-slop-detector.behavior.test.ts` around lines 282 - 290, Update
findUnsafeAssertions to inspect inline suppression directives on currentLine
before its early return for first-line input. Ensure `// eslint-disable-line`
suppresses findings such as `const x = value as any; // eslint-disable-line` on
line 1, while preserving the existing preceding-line suppression behavior.
Owner
Author
Accepted follow-ups (non-critical findings)Per review, these are accepted as non-critical — not fixed here:
|
…ection The AST migration (f0c4ce2) dropped the old regex-path logic that excluded test/spec/__tests__ files from unsafe_type_assertion findings in default (non-quiet) mode. Adding skipTests:true to the pattern definition restores this behavior — the existing decorator guard in the AST findings loop (analyzeFile, line 1202) already applies skipTests to AST findings. Package-lock bumped to match 1.0.29.
Owner
Author
…suppression P1: The AST migration (f0c4ce2) only recognized as unknown as T as a double assertion. Broaden condition to any nested AsExpression so that value as Foo as Bar continues to produce unsafe_double_type_assertion. P2: lineIsSuppressed returned false for line <= 1 before checking the current line. Reorder so the current line is always checked for inline suppressions (eslint-disable-line, @ts-ignore, etc.), and the previous line is checked only when it exists.
… regex Also removes orphaned blank lines left from the test-file exclusion block removal in f0c4ce2.
Adds tests verifying exact finding counts for chained assertions at every depth (1-chain->1, 2-chain->2, 3-chain->3, 4-chain->3) per the user's requirement that findings reflect the precise AST count with no deduplication. Also adds a comment on the first /(?!)/ sentinel regex explaining it is AST-driven and the pattern is a never-match placeholder.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ai-slop-detector.ts (1)
236-277: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the reported line when checking multiline suppressions.
Suppression is evaluated using
node.getStart()(sourceLine), but direct and chainedas anyfindings are reported usingnode.type.getStart(). In a multiline assertion,// eslint-disable-lineon the line containinganywill not suppress the finding. Check the effective finding line, or intentionally check both relevant lines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ai-slop-detector.ts` around lines 236 - 277, Update suppression handling for unsafe_type_assertion findings in the chained and direct assertion paths to use the effective reported line from node.type.getStart(sourceFile), rather than only node.getStart(sourceFile). Ensure multiline as any assertions can be suppressed by a directive on the line containing any, while preserving existing suppression behavior for unsafe_double_type_assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ai-slop-detector.ts`:
- Around line 489-498: Update the unsafe_type_assertion rule’s fix
recommendation to remove “as unknown as TargetType” and advise only validated
conversion or implementing a runtime type guard. Keep the rule’s detection
behavior, message, and other metadata unchanged.
- Around line 185-200: Update the suppression detection logic around the
current-line and previous-line checks to parse only actual comment directives,
ignoring directive-like text inside strings or trailing non-directive content.
Apply each ESLint directive according to its target-line semantics, ensuring
eslint-disable-line affects only its own line and does not suppress the
following line, while preserving next-line behavior for eslint-disable-next-line
and TypeScript suppressions.
- Around line 236-246: Update the chained-assertion detection around
ts.isAsExpression(node.expression) to unwrap any ParenthesizedExpression layers
before checking for an inner AsExpression. Ensure patterns such as (value as
unknown) as any are classified as unsafe_double_type_assertion with
assertionForm chained_as, while preserving the existing generic handling for
non-chained assertions.
In `@tests/ai-slop-detector.behavior.test.ts`:
- Around line 403-411: Assert result.status === 0 in the excluded test-fixture
case before checking stdout, and assert result.status === 1 in the detected
high-severity source-fixture case. Apply these assertions in
tests/ai-slop-detector.behavior.test.ts at lines 403-411 and 424-432,
respectively.
---
Outside diff comments:
In `@ai-slop-detector.ts`:
- Around line 236-277: Update suppression handling for unsafe_type_assertion
findings in the chained and direct assertion paths to use the effective reported
line from node.type.getStart(sourceFile), rather than only
node.getStart(sourceFile). Ensure multiline as any assertions can be suppressed
by a directive on the line containing any, while preserving existing suppression
behavior for unsafe_double_type_assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f277a4ed-4bfb-4d68-802e-f7b389701609
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
CHANGELOG.mdai-slop-detector.tstests/ai-slop-detector.behavior.test.ts
Comment on lines
+403
to
+411
| const result = spawnSync( | ||
| process.execPath, | ||
| ['--import', 'tsx', path.resolve(process.cwd(), 'ai-slop-detector.ts'), fixtureFile], | ||
| { encoding: 'utf-8', cwd: process.cwd() } | ||
| ); | ||
|
|
||
| try { | ||
| assert.ok(!result.stdout.includes('unsafe_type_assertion'), | ||
| `should not report as any in test files. stdout:"${result.stdout}"`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the expected CLI exit codes.
Both tests can pass when the process fails without producing the checked stdout text.
tests/ai-slop-detector.behavior.test.ts#L403-L411: assertresult.status === 0for the excluded test fixture.tests/ai-slop-detector.behavior.test.ts#L424-L432: assertresult.status === 1for the detected high-severity source fixture.
Proposed fix
try {
+ assert.equal(result.status, 0, `Expected exit code 0 but got ${result.status}. stderr:"${result.stderr}"`);
assert.ok(!result.stdout.includes('unsafe_type_assertion'),
`should not report as any in test files. stdout:"${result.stdout}"`);
@@
try {
+ assert.equal(result.status, 1, `Expected exit code 1 but got ${result.status}. stderr:"${result.stderr}"`);
assert.ok(result.stdout.includes('unsafe_type_assertion'),
`should report as any in non-test files. stdout:"${result.stdout}"`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = spawnSync( | |
| process.execPath, | |
| ['--import', 'tsx', path.resolve(process.cwd(), 'ai-slop-detector.ts'), fixtureFile], | |
| { encoding: 'utf-8', cwd: process.cwd() } | |
| ); | |
| try { | |
| assert.ok(!result.stdout.includes('unsafe_type_assertion'), | |
| `should not report as any in test files. stdout:"${result.stdout}"`); | |
| const result = spawnSync( | |
| process.execPath, | |
| ['--import', 'tsx', path.resolve(process.cwd(), 'ai-slop-detector.ts'), fixtureFile], | |
| { encoding: 'utf-8', cwd: process.cwd() } | |
| ); | |
| try { | |
| assert.equal(result.status, 0, `Expected exit code 0 but got ${result.status}. stderr:"${result.stderr}"`); | |
| assert.ok(!result.stdout.includes('unsafe_type_assertion'), | |
| `should not report as any in test files. stdout:"${result.stdout}"`); |
📍 Affects 1 file
tests/ai-slop-detector.behavior.test.ts#L403-L411(this comment)tests/ai-slop-detector.behavior.test.ts#L424-L432
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/ai-slop-detector.behavior.test.ts` around lines 403 - 411, Assert
result.status === 0 in the excluded test-fixture case before checking stdout,
and assert result.status === 1 in the detected high-severity source-fixture
case. Apply these assertions in tests/ai-slop-detector.behavior.test.ts at lines
403-411 and 424-432, respectively.
… anchoring - Fix: safe_type_assertion guidance no longer recommends 'as unknown as TargetType', which is itself flagged by unsafe_double_type_assertion - Fix: Multiline as any suppression now checks the 'any' token line, not the outer 'as' expression line - Updated fixture comment and CHANGELOG with accurate description of test-file exclusion scope
This was referenced Jul 27, 2026
Owner
Author
Follow-up issues created from review commentsThese items are tracked for validation and implementation in separate issues. They are not release blockers for 1.0.29.
Existing related issues:
|
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
unsafe_type_assertion(as any) from line-based regex to TypeScript Compiler API AST detection, following the same pattern as the existingunsafe_double_type_assertion,unsafe_object_assertion, andunsafe_array_assertionrules.Changes
as anyassertions are now detected by the AST visitor infindUnsafeAssertions()instead of the generic regex loopas anyare no longer reported — the AST naturally distinguishes syntax from textunsafe_type_assertionremains unchanged — existingseverityOverridesconfigurations continue to workTesting
@ts-expect-error,eslint-disable-next-line, and.d.tsexclusionfindUnsafeAssertionsexecutionVersion
1.0.29CodeAnt-AI Description
Detect unsafe
as anyassertions accurately in TypeScript codeWhat Changed
as anyassertions are now detected from TypeScript syntax, including nested and multiline expressionsunsafe_type_assertionconfiguration keys remain supportedImpact
✅ Fewer false-positive unsafe assertion warnings✅ Reliable detection of nested and multiline assertions✅ Unchanged severity override compatibility💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Summary by CodeRabbit
as any, including nested, multiline, and chained forms with accurate source positioning.eslint-disable-line/@ts-ignoreon the first line and improved multiline suppression anchoring.as anyis no longer reported from comments/strings/template literals or.d.tsinputs; guidance text updated to avoid recommending chained assertions.