Repository navigation
Accept only @throws and @doesNotThrow in reanalyze - #9210
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Changelog💥 Breaking Change
Semantic Diff1 commit · Full page Accept only
|
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9210 +/- ##
=======================================
Coverage 81.95% 81.96%
=======================================
Files 456 456
Lines 65612 65607 -5
=======================================
+ Hits 53775 53776 +1
+ Misses 11837 11831 -6
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9210 |
fd46de2 to
2de3f55
Compare
Why:
12.0.0-beta.14 deprecated `@raises` and `@doesNotRaise` in favour of
`@throws` and `@doesNotThrow` (#7932). v13 drops the old spellings,
the undeprecated `@throw` synonym, and the `@doNotThrow`/case variants,
so one spelling remains for each annotation.
An old spelling is now an unknown attribute, so the code it annotated is
reported like unannotated code. The compiler's own `[@doesNotRaise]` and
`[@@raises]` annotations become `[@doesNotThrow]` and `[@@throws]`.
Reanalyze modified (old annotation spellings):
Example:
@raises(Not_found)
let oldRaisesAnnotation = () => throw(Not_found)
Before (reanalyze): no issue
After (reanalyze):
Exception Analysis
ExnOldSpellings.res:5:5-23
oldRaisesAnnotation might throw Not_found (ExnOldSpellings.res:5:32) and is not annotated with @throws(Not_found)
Review: Why no dedicated warning for an old spelling? The missing-annotation report already names the `@throws(...)` to write, and recognizing the old names only to reject them would keep them in the analysis.
Breaking: `@raises`, `@raise`, `@throw`, `@doesNotRaise`, `@doNotRaise`, `@doNotThrow` and the lower-case variants are ignored; write `@throws` and `@doesNotThrow`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gGhHjzBTXqSo8Dckpb
Signed-off-by: Christoph Knittel <ck@cca.io>
2de3f55 to
3368127
Compare
Changelog💥 Breaking Change
Semantic Diff1 commit · Full page Accept only
|
✅ Compiler benchmarksAllocated wordsIn total: 682,344,971 → 682,345,054 (±0.00%). Generated projects: 923,214,162 → 923,213,879 (±0.00%). None of the 265 benchmarks changed by 0.1% or more. InstructionsIn total: 9,666,072,926 → 9,666,072,926 (±0.00%). None of the 33 benchmarks changed by 0.1% or more. Output sizeIn total: 25,812,251 → 25,812,251 (±0.00%). None of the 13 benchmarks changed by 0.1% or more. Pull request 3368127 compared with base 32116b5 in this run. |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9210 |
cristianoc
left a comment
There was a problem hiding this comment.
Dot, Cristiano's assistant, reviewing at his request.
Reviewed the canonical @throws / @doesNotThrow handling, migration of internal annotations, completion entries, and old-spelling regression expectations. The removal is consistent with the documented v13 breaking change; no blocking findings. Exact-head CI is green. Static review and CI inspection only; I did not rerun the compiler/tests locally.
#7932 deprecated
@raisesand@doesNotRaisein 12.0.0-beta.14 and made@throwsand@doesNotThrowthe documented spellings. Reanalyze still accepts the old names, plus several synonyms nobody documents:@raise,@throw(added in #7521 next to@raise, never deprecated and never offered by completion), and a family of@doNotThrow/@doNotRaisevariants with lower-case and typo spellings such as@doesnotthrowand@DoesNoThrow. For v13, this keeps one spelling per annotation.Exception analysis now recognizes only
@throwsand@doesNotThrow. Editor completion no longer offers@raisesand@doesNotRaise. Every other spelling is an ordinary unknown attribute, and reanalyze ignores it. There is no dedicated warning for an old spelling. Code annotated with one is reported like unannotated code, and the missing-annotation report already names the@throws(...)to add. A separate warning would mean keeping the old names in the analysis only to reject them.The compiler's own OCaml sources used the old spellings: 72
[@doesNotRaise]and 15[@@raises ...]annotations in the syntax, gentype, ext and analysis code. They now read[@doesNotThrow]and[@@throws ...]. CONTRIBUTING.md now says@throwswhere it said@throw. Test fixtures that only contain the old words as code stay as they are: the syntax benchmark data and the printer fixturebinary.res.Tests
tests/analysis_tests/tests-reanalyze/deadcode/src/exception/ExnOldSpellings.resuses@raises,@raise,@throw,@doesNotRaise,@doNotRaise,@doNotThrowand@doesnotthrow.expected/exception.txtnow reports each annotated value as throwing and not annotated with@throws.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gGhHjzBTXqSo8Dckpb