super_errors: expand warning + error fixture coverage#8429
Open
JonoPrest wants to merge 9 commits into
Open
Conversation
Adds one-fixture-per-printed-branch snapshots for warnings that previously had zero coverage: warning 12 - redundant sub-pattern warning 33 - unused open warning 34 - unused type declaration warning 35 - unused for-loop index warning 37 - unused constructor (all 3 message branches) warning 38 - unused extension constructor / exception (2 branches each) warning 39 - unused rec flag warning 60 - unused module warning 101 - unused @bs.* attribute Each fixture uses a private module signature to ensure the warning fires consistently regardless of whether the file has consumers. Naming convention: warning_NN_<short_description>.res so coverage gaps stay easy to grep for.
Adds coverage for Nonoptional_label, the warning that fires when a
caller uses `~x=?` syntax against a non-optional labeled argument.
Inventory finding: warnings 16 (Unerasable_optional_argument) and 48
(Eliminated_optional_arguments) are effectively dead in current ReScript.
- 16 is raised inside type_function (typecore.ml:3525) but is
explicitly disabled at the start of the same function
(typecore.ml:3479) with the comment "Disable
Unerasable_optional_argument for uncurried functions". Since
ReScript is fully uncurried, the warning never fires.
- 48 has the variant declared in warnings.ml but no
`prerr_warning Eliminated_optional_arguments` call exists
anywhere in the codebase, so it can never be raised.
No fixtures written for 16 or 48.
…cl / bs_* warnings
Adds one-fixture-per-printed-branch coverage for previously untested
warnings, plus extra branches for warnings whose printed text varies
with payload shape:
warning 23 - useless record `with` spread (all fields overridden)
warning 28 - wildcard pattern on a constant constructor
warning 30 - duplicate constructor in mutually-recursive type defs
warning 41 - ambiguous field labels across multiple record types
warning 44 - open shadows an existing value identifier
warning 45 - open shadows a constructor (also fires 44 because
the parent type is shadowed at the same time; both
are captured)
warning 47 - illegal payload on @inline attribute
warning 52 - fragile literal pattern (matching on Invalid_argument
payload string)
warning 53 - misplaced @inline on a non-constant let binding
(the constant case is consumed by bs_builtin_ppx
inline-const transform before it reaches translcore)
warning 54 - duplicated @inline attribute
warning 57 - ambiguous or-pattern variables under guard, both
single-var and multi-var printed branches
warning 103 - @string redundant on a payload-less polymorphic
variant in an external
warning 104 - @deriving applied to a non-applicable type
(`accessors` requires a record type)
Inventory notes (no fixtures added):
- warnings 5 (Partial_application) and 10 (Statement_type): these
are now handled as enriched type errors via the `Statement
FunctionCall` type-clash context in error_message_utils.ml,
rather than as warnings. The `prerr_warning` call sites in
typecore.ml still exist but the user-visible behaviour is a
type error, not a warning.
- warnings 14 (Illegal_backslash), 29 (Eol_in_string),
105 (Bs_fragile_external), 106 (Bs_unimplemented_primitive),
108 (Bs_uninterpreted_delimiters), 50 (Bad_docstring):
no `prerr_warning` calls for these anywhere in the compiler.
Variant exists in warnings.ml but is unreachable.
- warning 109 None branch (no help text) is raised only when
polymorphic-variant tag unification fails (typecore.ml:4524).
Hard to trigger from user code; the two reachable branches
(Some FunctionCall, Some Other) are already covered by
existing top_level_*_not_unit fixtures.
- warning 8 empty-hint branch ("") is a `try ... with _ -> ""`
defensive fallback in parmatch.ml when example-pattern
printing raises. Not reachable from normal user code.
Constraint clause on a GADT (variant where at least one constructor has a result type ascription). ReScript GADT syntax requires `type rec` and `t<'a>` parametric notation; the OCaml `type _ t` form isn't accepted by the ReScript parser. The fixture also triggers a follow-up type error because the constraint `'a = int` makes `t<bool>` unsatisfiable — captured in the snapshot as part of documenting the full diagnostic flow.
…evious batch
New error fixtures:
multiply_bound_variable.res - `let (x, x) = ...` (pattern
binds same name twice)
label_multiply_defined_literal.res - `{a: 1, a: 2}` record literal
with duplicate label
illegal_letrec_pat.res - `let rec (x, y) = ...`
(let rec LHS must be a variable)
exception_pattern_below_toplevel.res - `Some(exception Not_found)`
(exception patterns must be at
the top of a switch case)
or_pattern_type_clash.res - `| 1 | "hello" => ...` on an
`int`-typed scrutinee
orpat_vars_unbalanced.res - `| Some(n) | None => ...`
(variable bound on only one
side of an or-pattern)
Also reformats the previous warning batch via `make format`:
- `when` → `if` for switch guards (modern ReScript syntax)
- layout fixes for warning_53/57/62 to match the formatter
- snapshots regenerated against the formatted fixtures
Inventory finding: char/int interval patterns (`'5' .. '0'`) parse
silently in current ReScript — Invalid_interval (typecore Error
variant) appears to be unreachable from the surface syntax. Skipped.
repeated_type_parameter.res - `type t<'a, 'a>` (param name
used twice in the param list)
duplicate_variant_constructor.res - `type t = Foo | Foo`
type_abbrev_missing_rec.res - `type t = t` without `rec`,
which surfaces as Unbound_type
with a "did you mean type rec?"
hint rather than as a cycle
recursive_type_abbrev_cycle.res - `type rec t = t` (true cyclic
abbrev, no indirection)
unbound_type_var.res - `type t = ('a, int)` (uses 'a
without declaring it)
record_literal_missing_fields.res - `let r: t = {}` where `t` has
a required field (this fires
Labels_missing, not the
Empty_record_literal variant
which appears to be unreachable
for record-typed scrutinees)
duplicate_module_in_scope.res - two `module M = {…}` declarations
in the same structure (typemod
Repeated_name)
type_arity_mismatch.res - `array<int, string>`
(wrong number of type args)
typetexp_unbound_type_constructor.res - `let x: nonexistent_type = 1`
(typetexp variant; surfaces
with the same "did you mean
type rec?" hint as the
typedecl variant)
apply_wrong_label.res - calling a labeled-arg
function with the wrong
label name
too_many_arguments.res - passing 3 unlabelled args
to a 2-arg function
invalid_type_variable_name.res - `'_invalid` (type variable
starting with underscore
is not allowed)
functor_apply_arg_mismatch.res - applying a functor with a
module that doesn't satisfy
the parameter signature
(exercises includemod's
Module_type_mismatch path)
duplicated_bs_deriving.res - two `@deriving(...)` attributes
on the same type declaration
conflicting_ffi_attributes.res - `@val @send` on the same external
(these FFI annotations are
mutually exclusive)
Contributor
Author
|
@codex review |
Member
|
Great stuff @JonoPrest! |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ 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". |
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
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
2 participants
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
Adds 50 fixtures to
tests/build_tests/super_errors/covering compiler warnings (29) and errors (21). Drives the existing snapshot framework — shells out tobsc, diffs stderr — so tests are implementation-independent.Coverage moved from 49.79% → 50.83%, with bigger wins on the warning/error modules:
warnings.mlincludemod.mltypetexp.mltypedecl.mltypemod.mltypecore.mlScope
warnings.mlnow has a fixture, with one fixture per printed message branch (e.g. warning 37 has 3 — basic/pattern-only/private — matching the three arms inWarnings.message).Warning fixtures use
warning_NN_<description>.resso coverage gaps stay greppable.Dead-warning findings (no fixtures added, documented in commits)
type_functionfor uncurried).prerr_warningcall site: 1, 2, 14, 29, 48, 50, 105, 106, 108.