06 Dec 2018

Yak Shaving in F♭

Following on from my introduction to clippy lints, this week I am beginning my journey of actually implementing a clippy lint.

As a refresher, I am implementing rust-lang/rust-clippy#1673:

To summarize, if the type has a get_foo method we should suggest naming it foo instead to follow the API Guidelines for Rust getter name conventions except for cases of:

get

get_mut

get_unchecked

get_unchecked_mut

get_ref

This should be enough to get me started on a style lint for this convention, I should have some time over the next couple days to start digging into this.

`clippy`’s Author Lint

In typical TDD fashion, I want to start with the test case, the code I want to lint against, so I can test my implementation and drive design. I’ve started off with the simple case of detecting the invalid style rather than worrying about whitelisting the exceptions identified.

pub struct MyStruct {
    id: u32
}

impl MyStruct {
    pub fn get_id(&self) -> u32 {
        self.id
    }
}

fn main() {
   let s = MyStruct { id: 42 };

   #[clippy::author]
   let id = s.get_id();
}

I’ve also added the #[clippy::author] annotation as suggested by clippy’s contributing documentation to generate a starting point for the lint.

Next, I have to run the test to produce a .stdout file with the code generated by the #[clippy::author] lint. The instructions say:

If the command was executed successfully, you can copy the code over to where you are implementing your lint.

Let’s try it out:

$ TESTNAME=ui/getter_prefix cargo test --test compile-test
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running target/debug/deps/compile_test-f89d0316ceade355

running 1 test

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 29 filtered out


running 1 test
test [ui] ui/getter_prefix.rs ... FAILED

failures:

---- [ui] ui/getter_prefix.rs stdout ----
normalized stdout:
if_chain! {
    if let StmtKind::Decl(ref decl, _) = stmt.node
    if let DeclKind::Local(ref local) = decl.node;
    if let Some(ref init) = local.init
    if let ExprKind::MethodCall(ref method_name, ref generics, ref args) = init.node;
    // unimplemented: `ExprKind::MethodCall` is not further destructured at the moment
    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
    if name.node.as_str() == "id";
    then {
        // report your lint here
    }
}


expected stdout:


diff of stdout:

+if_chain! {
+    if let StmtKind::Decl(ref decl, _) = stmt.node
+    if let DeclKind::Local(ref local) = decl.node;
+    if let Some(ref init) = local.init
+    if let ExprKind::MethodCall(ref method_name, ref generics, ref args) = init.node;
+    // unimplemented: `ExprKind::MethodCall` is not further destructured at the moment
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
+    if name.node.as_str() == "id";
+    then {
+        // report your lint here
+    }
+}
+

The actual stdout differed from the expected stdout.
Actual stdout saved to /Users/scrust/devel/rust-clippy/target/debug/test_build_base/getter_prefix.stdout
To update references, run this command from build directory:
tests/ui/update-references.sh '/Users/scrust/devel/rust-clippy/target/debug/test_build_base' 'getter_prefix.rs'

error: 1 errors occurred comparing output.
status: exit code: 0
command: "target/debug/clippy-driver" "tests/ui/getter_prefix.rs" "-L" "/Users/scrust/devel/rust-clippy/target/debug/test_build_base" "--target=x86_64-apple-darwin" "-C" "prefer-dynamic" "-o" "/Users/scrust/devel/rust-clippy/target/debug/test_build_base/getter_prefix.stage-id" "-L" "target/debug" "-L" "target/debug/deps" "-Dwarnings" "-L" "/Users/scrust/devel/rust-clippy/target/debug/test_build_base/getter_prefix.stage-id.aux" "-A" "unused"
stdout:
------------------------------------------
if_chain! {
    if let StmtKind::Decl(ref decl, _) = stmt.node
    if let DeclKind::Local(ref local) = decl.node;
    if let Some(ref init) = local.init
    if let ExprKind::MethodCall(ref method_name, ref generics, ref args) = init.node;
    // unimplemented: `ExprKind::MethodCall` is not further destructured at the moment
    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
    if name.node.as_str() == "id";
    then {
        // report your lint here
    }
}

------------------------------------------
stderr:
------------------------------------------

------------------------------------------

thread '[ui] ui/getter_prefix.rs' panicked at 'explicit panic', /Users/scrust/.cargo/registry/src/github.com-1ecc6299db9ec823/compiletest_rs-0.3.17/src/runtest.rs:2553:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [ui] ui/getter_prefix.rs

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 225 filtered out

test compile_test ... FAILED

failures:

---- compile_test stdout ----
thread 'compile_test' panicked at 'Some tests failed', /Users/scrust/.cargo/registry/src/github.com-1ecc6299db9ec823/compiletest_rs-0.3.17/src/lib.rs:89:22


failures:
    compile_test

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--test compile-test'

Wow that’s a lot out output. More importantly, I don’t actually know if it worked. I see

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 29 filtered out

but then I see

The actual stdout differed from the expected stdout.
Actual stdout saved to /Users/scrust/devel/rust-clippy/target/debug/test_build_base/getter_prefix.stdout

and

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 225 filtered out

test compile_test ... FAILED

I also don’t see any generated .stdout file, so I’m going to assume there’s something wrong with my test case.

If I remove the author lint tag, I get a different result:

$ TESTNAME=ui/getter_prefix cargo test --test compile-test
    Finished dev [unoptimized + debuginfo] target(s) in 0.16s
     Running target/debug/deps/compile_test-f89d0316ceade355

running 1 test

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 29 filtered out


running 1 test
test [ui] ui/getter_prefix.rs ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 225 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out

test compile_test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

This all seems to pass, but I still don’t see a generated .stdout file, and I’ve also lost any output the #[clippy:author] annotation would have given me.

I noticed a line from the earlier, possibly failed, test run:

To update references, run this command from build directory: tests/ui/update-references.sh ‘/Users/scrust/devel/rust-clippy/target/debug/test_build_base’ ‘getter_prefix.rs’

What does this do? What’s a “reference”? clippy’s documentation doesn’t seem to mention tests/ui/update-references.sh at all, though there is a mention of tests/ui/update-all-references.sh which seems to update all of the existing .stderr files that drive the UI tests.

It turns out that running tests/ui/update-references.sh is necessary to actually write the .stdout file. I wasn’t expecting this extra step because the way the instructions are phrased I though running the test would generate the .stdout file automatically. For the test I wrote, #[clippy::author] generated a .stdout file with the following code:

if_chain! {
    if let StmtKind::Decl(ref decl, _) = stmt.node
    if let DeclKind::Local(ref local) = decl.node;
    if let Some(ref init) = local.init
    if let ExprKind::MethodCall(ref method_name, ref generics, ref args) = init.node;
    // unimplemented: `ExprKind::MethodCall` is not further destructured at the moment
    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
    if name.node.as_str() == "id";
    then {
        // report your lint here
    }
}

I’m not at all familiar with the if_chain! macro or any of these datatypes, so I definitely have some reading to do so I can understand what this snippet actually does. I do see if name.node.as_str() == "id"; which seems to match the id field on MyStruct which is about the only piece I understand without delving deeper.

Out of curiosity after reading a number of other lint tests, I decided to update main and added an assert statement:

fn main() {
   let s = MyStruct { id: 42 };

    #[clippy::author]
    let id = s.get_id();

    assert_eq!(id, 42);
}

and got yet another different result:

$ TESTNAME=ui/getter_prefix cargo test --test compile-test
    Finished dev [unoptimized + debuginfo] target(s) in 0.16s
     Running target/debug/deps/compile_test-f89d0316ceade355

running 1 test

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 29 filtered out


running 1 test
test [ui] ui/getter_prefix.rs ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 225 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out


running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out

test compile_test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

I’m not sure the #[clippy::author] annotation is going to help me too much in implementing this lint. Right now I don’t know enough about clippy or the datatypes it uses to make heads or tails of the generated code, and the results of my test are inconsistent depending on various combinations of assert_eq! and the author annotation. This definitely calls for more research, so until next week it looks like I’ll be getting to grips with some of rustc’s compiler internals.

stuart crust

a constant work in progress

06 Dec 2018

`clippy`’s Author Lint

stuart crust

a constant work in progress

06 Dec 2018

clippy’s Author Lint

`clippy`’s Author Lint