Improve contract macro diagnostics

[robjtede]

Summary

• return compile errors instead of panics for invalid contract macro targets
• report misplaced contract descriptions directly
• add an MSRV-gated trybuild fixture for malformed predicate syntax

Validation

• cargo +1.65.0 test --test ui
• just clippy
• just test

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for contract macros so invalid input now produces clear compile-time diagnostics instead of panics.
  • Enforced that contract descriptions must be placed last, with a direct error message when they are not.
  • Added better feedback for malformed predicate syntax and unsupported attribute usage on non-function items.

• Tests

  • Updated and added UI failure tests to cover the new error messages and parsing behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Attribute macros ensures, invariant, and requires now use syn::parse2 instead of syn::parse_quote!, returning compile-time errors alongside original tokens on parse failure. invariant replaces a panic with a proper error for unsupported item kinds. parse_attributes validates that contract descriptions appear last, using new helper functions. UI test fixtures are updated accordingly, plus a new invalid-predicate-syntax test is added.

Changes

Macro error handling and validation

Layer / File(s)	Summary
Fallible parsing in requires and ensures src/implementation/requires.rs, src/implementation/ensures.rs	Both macros now parse toks via syn::parse2 and return a compile-error token stream combined with original tokens on failure, instead of using syn::parse_quote!.
Fallible parsing and item validation in invariant src/implementation/invariant.rs	invariant switches to syn::parse2 with explicit error handling and replaces unimplemented! with a syn::Error for item kinds other than Fn/Impl.
Contract description positional validation src/implementation/parse.rs	parse_attributes detects string literals appearing before the last argument and emits a compile_error; new is_string_lit and string_lit_value helpers extract the description from the final expression.
UI failure test fixtures tests/ui/fail/description_must_be_last.stderr, tests/ui/fail/invariant_on_struct.stderr, tests/ui/fail/requires_on_struct.stderr, tests/ui/fail/invalid_predicate_syntax.rs, tests/ui/fail/invalid_predicate_syntax.stderr	Existing stderr expectations updated to match new diagnostics (no more panics), and a new UI test with #[requires(x >)] and its expected stderr is added.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• x52dev/contracts#115: Updates the same tests/ui/fail/description_must_be_last.* fixture pair and related UI test harness that this PR's diagnostic changes directly affect.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: improving contract macro diagnostics and error handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex-add-trybuild-compile-fail-tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[robjtede]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (2)

src/implementation/requires.rs (1)

13-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Solid fix: panics replaced with compile errors. Consider matching invariant's explicit item-kind handling for a clearer message.

Right now, #[requires] on a non-fn item (struct/enum/etc.) surfaces whatever generic message syn produces for ItemFn parse failure (e.g. "expected fn"), rather than a purpose-built diagnostic like invariant's "the #[{}] attribute only works on functions and impl blocks". Since this is the same category of misuse the PR is targeting, using an explicit Item-based check (as invariant.rs does) would give consistent, clearer diagnostics here too.

♻️ Suggested approach

-    let func: ItemFn = match syn::parse2(toks.clone()) {
-        Ok(func) => func,
-        Err(err) => {
-            let error = err.to_compile_error();
-            return quote::quote! {
-                `#error`
-                `#toks`
-            };
-        }
-    };
+    let item: syn::Item = match syn::parse2(toks.clone()) {
+        Ok(item) => item,
+        Err(err) => {
+            let error = err.to_compile_error();
+            return quote::quote! {
+                `#error`
+                `#toks`
+            };
+        }
+    };
+
+    let func = match item {
+        syn::Item::Fn(func) => func,
+        item => {
+            let error = syn::Error::new_spanned(
+                &item,
+                "the #[requires] attribute only works on functions",
+            )
+            .to_compile_error();
+            return quote::quote! {
+                `#error`
+                `#item`
+            };
+        }
+    };

Same applies to ensures.rs.

🤖 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 `@src/implementation/requires.rs` around lines 13 - 22, The #[requires] and
#[ensures] attribute handlers currently rely on parsing as ItemFn, which
produces generic syn errors for non-function items; update the entry points in
requires.rs and ensures.rs to do explicit item-kind matching like invariant.rs,
so misuse on structs/enums/etc. returns a purpose-built compile error such as
“the #[...] attribute only works on functions and impl blocks.” Keep the
existing compile-error flow, but replace the ItemFn-only parse failure path with
a clearer Item-based check and diagnostic.

src/implementation/parse.rs (1)

40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify Option flattening with and_then.

.map(string_lit_value).unwrap_or(None) is equivalent to .and_then(string_lit_value).

♻️ Proposed simplification

-    let desc = conds.last().map(string_lit_value).unwrap_or(None);
+    let desc = conds.last().and_then(string_lit_value);

🤖 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 `@src/implementation/parse.rs` at line 40, Simplify the `Option` handling in
the `parse.rs` logic by replacing the
`conds.last().map(string_lit_value).unwrap_or(None)` pattern with
`and_then(string_lit_value)`. Update the `desc` assignment in the affected
parsing code to use the flatter combinator directly, keeping the same behavior
while making the expression shorter and clearer.

🤖 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.

Nitpick comments:
In `@src/implementation/parse.rs`:
- Line 40: Simplify the `Option` handling in the `parse.rs` logic by replacing
the `conds.last().map(string_lit_value).unwrap_or(None)` pattern with
`and_then(string_lit_value)`. Update the `desc` assignment in the affected
parsing code to use the flatter combinator directly, keeping the same behavior
while making the expression shorter and clearer.

In `@src/implementation/requires.rs`:
- Around line 13-22: The #[requires] and #[ensures] attribute handlers currently
rely on parsing as ItemFn, which produces generic syn errors for non-function
items; update the entry points in requires.rs and ensures.rs to do explicit
item-kind matching like invariant.rs, so misuse on structs/enums/etc. returns a
purpose-built compile error such as “the #[...] attribute only works on
functions and impl blocks.” Keep the existing compile-error flow, but replace
the ItemFn-only parse failure path with a clearer Item-based check and
diagnostic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 94e16527-1adf-4518-bcfe-32d82d50e139

📥 Commits

Reviewing files that changed from the base of the PR and between 2c6109a and 8055bcf.

📒 Files selected for processing (9)

• src/implementation/ensures.rs
• src/implementation/invariant.rs
• src/implementation/parse.rs
• src/implementation/requires.rs
• tests/ui/fail/description_must_be_last.stderr
• tests/ui/fail/invalid_predicate_syntax.rs
• tests/ui/fail/invalid_predicate_syntax.stderr
• tests/ui/fail/invariant_on_struct.stderr
• tests/ui/fail/requires_on_struct.stderr