Skip to content

Commit

Permalink
[3/n] [daft-derive] attempt to better annotate semantic errors
Browse files Browse the repository at this point in the history
Add some span info to try and annotate semantic errors better (such as
highlighting the name of the field that's not diffable).

This isn't perfect for now because we still produce some call-site errors, but
at least we produce one error that's attributed directly to the field name so
rust-analyzer should be happy. I tried adding more bounds but ran into
rust-lang/rust#48214.

Reviewers: andrewjstone

Reviewed By: andrewjstone

Pull Request: #60
  • Loading branch information
sunshowers authored Mar 10, 2025
1 parent f4fc719 commit 7cb7e7a
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 14 deletions.
37 changes: 34 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,38 @@ jobs:
build-and-test:
name: Build and test
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
fail-fast: false
env:
RUSTFLAGS: -D warnings
CARGO_TERM_COLOR: always
steps:
- uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4
- name: Install toolchain in rust-toolchain.toml
# GitHub Actions has an older rustup as of 2025-03-07 which doesn't accept
# `rustup toolchain install` with no arguments. Update rustup first.
run: rustup self update && rustup toolchain install
- uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2
- uses: taiki-e/install-action@cargo-hack
- uses: taiki-e/install-action@just
- uses: taiki-e/install-action@nextest
- name: Build
run: just powerset build
- name: Run tests
run: just powerset nextest run
- name: Doctests
run: just powerset test --doc

build-and-test-msrv:
name: Build and test (MSRV)
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
# 1.81 is the MSRV
rust-version: ["1.81", stable]
rust-version: ["1.81"]
fail-fast: false
env:
RUSTFLAGS: -D warnings
Expand All @@ -54,7 +81,11 @@ jobs:
- uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2
- uses: taiki-e/install-action@cargo-hack
- uses: taiki-e/install-action@just
- uses: taiki-e/install-action@nextest
- name: Build
run: just powerset build
- name: Test
run: just powerset test
# We don't run ui_test on the MSRV since compiler output varies by version.
- name: Test without ui_test
run: just powerset nextest run -E 'not binary(ui_test)'
- name: Doctests
run: just powerset test --doc
4 changes: 3 additions & 1 deletion Justfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
set positional-arguments

# Note: help messages should be 1 line long as required by just.

# Print a help message.
Expand All @@ -8,7 +10,7 @@ help:
powerset *args:
# Group third-party implementation features to avoid a full combinatorial
# explosion -- we assume that they build independent of each other.
cargo hack --feature-powerset --workspace {{args}} --group-features newtype-uuid1,oxnet01,uuid1 --ignore-unknown-features
cargo hack --feature-powerset --workspace "$@" --group-features newtype-uuid1,oxnet01,uuid1 --ignore-unknown-features

# Build docs for crates and direct dependencies
rustdoc *args:
Expand Down
19 changes: 10 additions & 9 deletions daft-derive/src/internals/imp.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use super::error_store::{ErrorSink, ErrorStore};
use proc_macro2::{Span, TokenStream};
use quote::{quote, ToTokens};
use quote::{quote, quote_spanned, ToTokens};
use syn::{
parse_quote, parse_str, visit::Visit, Attribute, Data, DataStruct,
DeriveInput, Expr, Field, Fields, GenericParam, Generics, Index, Lifetime,
LifetimeParam, Path, Token, WhereClause, WherePredicate,
parse_quote, parse_quote_spanned, parse_str, spanned::Spanned,
visit::Visit, Attribute, Data, DataStruct, DeriveInput, Expr, Field,
Fields, GenericParam, Generics, Index, Lifetime, LifetimeParam, Path,
Token, WhereClause, WherePredicate,
};

pub struct DeriveDiffableOutput {
Expand Down Expand Up @@ -563,11 +564,11 @@ impl DiffFields {
let mut f = f.clone();

f.ty = if config.mode == FieldMode::Leaf {
parse_quote! {
parse_quote_spanned! {f.span()=>
#daft_crate::Leaf<&#lt #ty>
}
} else {
parse_quote! {
parse_quote_spanned! {f.span()=>
<#ty as #daft_crate::Diffable>::Diff<#lt>
}
};
Expand All @@ -591,7 +592,7 @@ impl DiffFields {
trait_bound: &syn::TraitBound,
) -> WhereClause {
let predicates = self.types().map(|ty| -> WherePredicate {
parse_quote! {
parse_quote_spanned! {ty.span()=>
#ty: #trait_bound
}
});
Expand Down Expand Up @@ -627,14 +628,14 @@ fn generate_field_diffs(
}
};
if config.mode == FieldMode::Leaf {
quote! {
quote_spanned! {f.span()=>
#field_name: #daft_crate::Leaf {
before: &self.#field_name,
after: &other.#field_name
}
}
} else {
quote! {
quote_spanned! {f.span()=>
#field_name: #daft_crate::Diffable::diff(
&self.#field_name,
&other.#field_name
Expand Down
2 changes: 1 addition & 1 deletion daft-derive/tests/fixtures/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ These fixtures ensure that:

Each file in `invalid` is automatically picked up by the snapshot and UI tests.

Like with valid fixtures, `snapshot_test.rs` tests all macro invocations annotated with `#[derive(Diffable)]`.
Like with valid fixtures, `snapshot_test.rs` tests all macro invocations annotated with `#[derive(Diffable)]`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
struct MyStructDiff<'__daft> {
a: <i32 as ::daft::Diffable>::Diff<'__daft>,
b: <NonDiffable as ::daft::Diffable>::Diff<'__daft>,
}
impl<'__daft> ::std::fmt::Debug for MyStructDiff<'__daft>
where
<i32 as ::daft::Diffable>::Diff<'__daft>: ::std::fmt::Debug,
<NonDiffable as ::daft::Diffable>::Diff<'__daft>: ::std::fmt::Debug,
{
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
f.debug_struct(stringify!(MyStructDiff))
.field(stringify!(a), &self.a)
.field(stringify!(b), &self.b)
.finish()
}
}
impl<'__daft> ::std::cmp::PartialEq for MyStructDiff<'__daft>
where
<i32 as ::daft::Diffable>::Diff<'__daft>: ::std::cmp::PartialEq,
<NonDiffable as ::daft::Diffable>::Diff<'__daft>: ::std::cmp::PartialEq,
{
fn eq(&self, other: &Self) -> bool {
self.a == other.a && self.b == other.b
}
}
impl<'__daft> ::std::cmp::Eq for MyStructDiff<'__daft>
where
<i32 as ::daft::Diffable>::Diff<'__daft>: ::std::cmp::Eq,
<NonDiffable as ::daft::Diffable>::Diff<'__daft>: ::std::cmp::Eq,
{}
impl ::daft::Diffable for MyStruct {
type Diff<'__daft> = MyStructDiff<'__daft> where Self: '__daft;
fn diff<'__daft>(&'__daft self, other: &'__daft Self) -> MyStructDiff<'__daft> {
Self::Diff {
a: ::daft::Diffable::diff(&self.a, &other.a),
b: ::daft::Diffable::diff(&self.b, &other.b),
}
}
}
14 changes: 14 additions & 0 deletions daft-derive/tests/fixtures/invalid/struct-field-not-diffable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use daft::Diffable;

struct NonDiffable {}

#[derive(Diffable)]
struct MyStruct {
a: i32,
b: NonDiffable,
}

fn main() {
// MyStruct should still exist, even though the Diffable impl has errors.
let _ = MyStruct { a: 0, b: NonDiffable {} };
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
error[E0277]: the trait bound `NonDiffable: Diffable` is not satisfied
--> tests/fixtures/invalid/struct-field-not-diffable.rs:8:5
|
8 | b: NonDiffable,
| ^ the trait `Diffable` is not implemented for `NonDiffable`
|
= help: the following other types implement trait `Diffable`:
&'a T
()
(A, B)
(A, B, C)
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
(A, B, C, D, E, F, G)
and $N others

error[E0277]: the trait bound `NonDiffable: Diffable` is not satisfied
--> tests/fixtures/invalid/struct-field-not-diffable.rs:5:10
|
5 | #[derive(Diffable)]
| ^^^^^^^^ the trait `Diffable` is not implemented for `NonDiffable`
|
= help: the following other types implement trait `Diffable`:
&'a T
()
(A, B)
(A, B, C)
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
(A, B, C, D, E, F, G)
and $N others
= note: this error originates in the derive macro `Diffable` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `NonDiffable: Diffable` is not satisfied in `MyStructDiff<'__daft>`
--> tests/fixtures/invalid/struct-field-not-diffable.rs:5:10
|
5 | #[derive(Diffable)]
| ^^^^^^^^ within `MyStructDiff<'__daft>`, the trait `Diffable` is not implemented for `NonDiffable`
|
= help: the following other types implement trait `Diffable`:
&'a T
()
(A, B)
(A, B, C)
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
(A, B, C, D, E, F, G)
and $N others
note: required because it appears within the type `MyStructDiff<'__daft>`
--> tests/fixtures/invalid/struct-field-not-diffable.rs:5:10
|
5 | #[derive(Diffable)]
| ^^^^^^^^
note: required by a bound in `daft::Diffable::Diff`
--> $WORKSPACE/daft/src/diffable.rs
|
| / type Diff<'daft>
| | where
| | Self: 'daft;
| |____________________^ required by this bound in `Diffable::Diff`
= note: this error originates in the derive macro `Diffable` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `NonDiffable: Diffable` is not satisfied in `MyStructDiff<'__daft>`
--> tests/fixtures/invalid/struct-field-not-diffable.rs:5:10
|
5 | #[derive(Diffable)]
| ^^^^^^^^ within `MyStructDiff<'__daft>`, the trait `Diffable` is not implemented for `NonDiffable`
|
= help: the following other types implement trait `Diffable`:
&'a T
()
(A, B)
(A, B, C)
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
(A, B, C, D, E, F, G)
and $N others
note: required because it appears within the type `MyStructDiff<'__daft>`
--> tests/fixtures/invalid/struct-field-not-diffable.rs:5:10
|
5 | #[derive(Diffable)]
| ^^^^^^^^
= note: the return type of a function must have a statically known size
= note: this error originates in the derive macro `Diffable` (in Nightly builds, run with -Z macro-backtrace for more info)
8 changes: 8 additions & 0 deletions rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# We use a specific toolchain revision to ensure our tests - which
# occasionally depend on specific compiler output - remain stable
# for all developers until the toolchain is explicitly advanced.
# The intent is to keep this updated as new stable versions are relased.

[toolchain]
channel = "1.85.0"
profile = "default"

0 comments on commit 7cb7e7a

Please sign in to comment.