Intersection derive macro

[Serock3]

Add derive macro for the Intersection trait.

Fixes DES-725.

---

This change is 

[linear]

DES-725 Add derive macro for the `Intersection` trait

[hulthe]

Reviewable status: 0 of 17 files reviewed, 6 unresolved discussions (waiting on @Serock3)

---

gui/build-e2e.sh line 8 at r3 (raw file):

npm run build-test-executable
cp ../dist/mullvadvpn-app-e2e-tests "$HOME/.cache/mullvad-test/packages/app-e2e-tests-2024.1-beta2-dev-${COMMIT}_amd64-unknown-linux-gnu"
cp ../dist/mullvadvpn-app-e2e-tests "$HOME/.cache/mullvad-test/packages/app-e2e-tests-2024.1-beta2-dev-${COMMIT}_x86_64-unknown-linux-gnu"

did you mean to commit this?

---

mullvad-types/intersection-derive/Cargo.toml line 4 at r3 (raw file):

name = "intersection-derive"
description = "Derive macro for the `Intersection` trait"
version = "0.1.0"

Probably shouldn't have a version

---

mullvad-types/intersection-derive/src/lib.rs line 18 at r3 (raw file):

        syn::Data::Struct(data) => derive_for_struct(&input, data),
        syn::Data::Enum(_) => todo!(),
        syn::Data::Union(_) => todo!(),

These should ideally return a nice syn::Error rather than panicking, like the Tuple struct case below.

---

mullvad-types/intersection-derive/src/lib.rs line 36 at r3 (raw file):

        field_conversions.append_all(quote! {
            #name: self.#name.intersection(other.#name)?,

Macro-generated code should refer to items by their absolute paths, even when calling trait methods. Otherwise the generated code will be dependent on what items the macro caller has in scope.

Sadly, this is kind of ugly x)

Suggestion:

            #name: ::mullvad_types::Intersection::intersection(self.#name, other.#name)?,

---

mullvad-types/intersection-derive/src/lib.rs line 45 at r3 (raw file):

                Some(Self {
                    #field_conversions
                })

Suggestion:

        impl ::mullvad_types::Intersection for #my_type {
            fn intersection(self, other: Self) -> ::core::option::Option<Self> {
                ::core::option::Option::Some(Self {
                    #field_conversions
                })

---

mullvad-types/src/lib.rs line 132 at r3 (raw file):

                }
            }
        }

This macro should ideally also name items by their absolute paths, especially since it's macro_exported.

[Serock3]

Reviewable status: 0 of 17 files reviewed, 6 unresolved discussions (waiting on @hulthe)

---

mullvad-types/intersection-derive/Cargo.toml line 4 at r3 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Probably shouldn't have a version

True

---

mullvad-types/intersection-derive/src/lib.rs line 36 at r3 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Macro-generated code should refer to items by their absolute paths, even when calling trait methods. Otherwise the generated code will be dependent on what items the macro caller has in scope.

Sadly, this is kind of ugly x)

I thought that since I export the derive macro with the same name as the trait (e.g. if you use mullvad_types::Intersection, you get both which is a super strange namespace rule I had no idea about) that it would be ok. But I guess if you were to use use mullvad_types::Intersection as SomethingElse then it would break.

This is giving me issues though, I can't use the derive macro in the mullvad_types crate since it expects crate::Intersection::intersection. Google tells me that this is a hard problem to solve, potentially https://crates.io/crates/proc-macro-crate could do it, but that adds another dependency.

---

mullvad-types/src/lib.rs line 132 at r3 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

This macro should ideally also name items by their absolute paths, especially since it's macro_exported.

Luckily, for declarative macros you have the $crate syntax which just works.

---

gui/build-e2e.sh line 8 at r3 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

did you mean to commit this?

Nope, thanks for catching that!

[Serock3]

Reviewable status: 0 of 17 files reviewed, 6 unresolved discussions (waiting on @hulthe)

---

mullvad-types/intersection-derive/src/lib.rs line 18 at r3 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

These should ideally return a nice syn::Error rather than panicking, like the Tuple struct case below.

Done.

[MarkusPettersson98]

Reviewed 11 of 16 files at r1, 2 of 3 files at r2, 2 of 2 files at r4, 4 of 6 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @hulthe and @Serock3)

---

mullvad-relay-selector/src/relay_selector/mod.rs line 743 at r4 (raw file):

            // In the case where there is only one exit to choose from, we have to pick it before
            // the entry
            (exits, [entry]) if exits.contains(entry) => {

In the case where there is only one entry to choose from, we have to pick it before the exit*

Code quote:

            // In the case where there is only one exit to choose from, we have to pick it before
            // the entry
            (exits, [entry]) if exits.contains(entry) => {

---

mullvad-types/intersection-derive/Cargo.toml line 11 at r4 (raw file):

rust-version.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Nit: Probably don't need this comment :D

Code quote:

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

---

mullvad-types/intersection-derive/src/lib.rs line 1 at r6 (raw file):

//! This `proc-macro` crate exports the [`Intersection`] derive macro, see it's documentation for

Nit its

Code quote:

it's

[dlon]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @hulthe and @Serock3)

---

mullvad-types/src/lib.rs line 145 at r6 (raw file):

impl_intersection_partialeq!(relay_constraints::Ownership);
// NOTE: it contains an inner constraint
impl_intersection_partialeq!(relay_constraints::TransportPort);

I think you could just add derive(Intersection) to TransportPort, since there's an implementation for TransportProtocol.

[Serock3]

Reviewable status: 10 of 17 files reviewed, 7 unresolved discussions (waiting on @hulthe and @MarkusPettersson98)

---

mullvad-relay-selector/src/relay_selector/mod.rs line 743 at r4 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

In the case where there is only one entry to choose from, we have to pick it before the exit*

Whoops, fixed that

---

mullvad-types/intersection-derive/Cargo.toml line 11 at r4 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit: Probably don't need this comment :D

Nope, forgot to remove the auto generated comment

---

mullvad-types/intersection-derive/src/lib.rs line 45 at r3 (raw file):

                Some(Self {
                    #field_conversions
                })

I ended up leaving the relative Intersection reference, but added a comment about the https://crates.io/crates/proc-macro-crate crate if it causes problems in the future

[MarkusPettersson98]

Reviewed 7 of 7 files at r7, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe)

[MarkusPettersson98]

Reviewed all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe)

[MarkusPettersson98]

Reviewed 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe)

[dlon]

Reviewed all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe)