feat: enrich no-unsafe-type-assertion diagnostics#868
Conversation
How to use the Graphite Merge QueueAdd the label 0-merge to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
camc314
force-pushed
the
c/codex/issue-648-no-unsafe-type-assertion-diagnostics
branch
from
40e475e to
8af50a2
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the no-unsafe-type-assertion rule’s diagnostics by adding richer guidance (Help) and attaching labeled source ranges that show the original vs asserted types to make reports more actionable.
Changes:
- Add
Helptext to allno-unsafe-type-assertionrule messages. - Switch from
ReportNodetoReportDiagnosticand emitLabeledRangesfor the assertion expression/type. - Update rule-tester snapshots to include
Helpand labeled range renderings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/rules/no_unsafe_type_assertion/no_unsafe_type_assertion.go | Adds Help strings and emits enriched diagnostics with labeled ranges (original vs asserted type). |
| internal/rule_tester/snapshots/no-unsafe-type-assertion.snap | Updates snapshots to reflect Help lines and labeled range snippets. |
| e2e/snapshots/snapshot.test.ts.snap | Updates e2e snapshot output (currently appears inconsistent with the implemented rule changes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LabeledRanges: []rule.RuleLabeledRange{ | ||
| { | ||
| Label: "Original type: " + ctx.TypeChecker.TypeToString(expressionType), | ||
| Range: utils.TrimNodeTextRange(ctx.SourceFile, expression), | ||
| }, | ||
| { | ||
| Label: "Asserted type: " + ctx.TypeChecker.TypeToString(assertedType), | ||
| Range: utils.TrimNodeTextRange(ctx.SourceFile, typeAnnotation), | ||
| }, |
There was a problem hiding this comment.
For the isUnsafeExpressionAny / isUnsafeAssertedAny branches, the user-facing message can mention an intrinsic error any (via getAnyTypeName(sender)), but the labeled range here always prints TypeToString(expressionType) / TypeToString(assertedType), which renders as plain any in snapshots. That makes the new labeled ranges inconsistent with the diagnostic message. Consider passing the sender type (or a precomputed display string such as getAnyTypeName(sender)) into buildUnsafeTypeAssertionDiagnostic so the label matches the message in these cases.
| "kind": 0, | ||
| "message": { | ||
| "description": "Unsafe call of a(n) \`any\` typed value.", | ||
| "id": "unsafeCall", | ||
| }, | ||
| "range": { | ||
| "end": 245, | ||
| "pos": 222, | ||
| }, | ||
| "rule": "no-unsafe-call", | ||
| }, | ||
| { | ||
| "file_path": "fixtures/basic/rules/no-unsafe-call/index.ts", | ||
| "kind": 0, | ||
| "message": { | ||
| "description": "Unsafe member access .then on an \`any\` value.", | ||
| "id": "unsafeMemberExpression", | ||
| }, | ||
| "range": { | ||
| "end": 237, | ||
| "pos": 233, | ||
| }, | ||
| "rule": "no-unsafe-member-access", | ||
| }, | ||
| { | ||
| "file_path": "fixtures/basic/rules/no-unsafe-call/index.ts", | ||
| "kind": 0, | ||
| "message": { |
There was a problem hiding this comment.
This snapshot change adds message.help and labeled_ranges to a no-unsafe-enum-comparison diagnostic, but the rule currently reports via ctx.ReportNode(..., buildMismatchedConditionMessage()) and that message does not set Help nor any labeled ranges. Conversely, this PR adds Help + labeled ranges for no-unsafe-type-assertion diagnostics, but the e2e snapshot entries for fixtures/basic/rules/no-unsafe-type-assertion/index.ts still don’t show those new fields. Please regenerate/update the e2e snapshot from actual headless output so it matches the code changes (and avoid introducing fields for rules that don’t emit them).
camc314
force-pushed
the
c/codex/issue-648-no-unsafe-type-assertion-diagnostics
branch
from
8af50a2 to
1658624
Compare
No description provided.