add neg_multiply lint #862

Merged
merged 6 commits into from Apr 17, 2016

Conversation

Projects
None yet
2 participants
@llogiq
Collaborator

llogiq commented Apr 17, 2016

This fixes #784 and is one part of the results of the Rust Lint Workshop.

Yeah, I know, it's kinda underwhelming, but at least a few folks learned a little bit about writing lints.

cc @regexident @hannobraun

src/neg_multiply.rs
+ match (&l.node, &r.node) {
+ (&ExprUnary(..), &ExprUnary(..)) => (),
+ (&ExprUnary(ref neg, ref lit), _) =>
+ if neg == &UnNeg { check_mul(cx, e.span, lit, r) },

This comment has been minimized.

@mcarton

mcarton Apr 17, 2016

Collaborator

This could just be ExprUnary(UnNeg, …)

@mcarton

mcarton Apr 17, 2016

Collaborator

This could just be ExprUnary(UnNeg, …)

This comment has been minimized.

@llogiq

llogiq Apr 17, 2016

Collaborator

Yes. I just wanted to show match guards. 😄 But I guess I won't need them here.

@llogiq

llogiq Apr 17, 2016

Collaborator

Yes. I just wanted to show match guards. 😄 But I guess I won't need them here.

+ -1 * x;
+ //~^ ERROR Negation by multiplying with -1
+
+ -1 * -1; // should be ok

This comment has been minimized.

@mcarton

mcarton Apr 17, 2016

Collaborator

Why?

@mcarton

mcarton Apr 17, 2016

Collaborator

Why?

This comment has been minimized.

@llogiq

llogiq Apr 17, 2016

Collaborator

Because this isn't exactly "negating by multiplication". Linting this would only confuse users.

@llogiq

llogiq Apr 17, 2016

Collaborator

Because this isn't exactly "negating by multiplication". Linting this would only confuse users.

@mcarton

This comment has been minimized.

Show comment
Hide comment
@mcarton

mcarton Apr 17, 2016

Collaborator

part of the results of the Rust Lint Workshop

Nice 😄

Collaborator

mcarton commented Apr 17, 2016

part of the results of the Rust Lint Workshop

Nice 😄

llogiq added some commits Apr 17, 2016

src/neg_multiply.rs
+///
+/// **Why is this bad?** It's more readable to just negate.
+///
+/// **Known problems:** This assumes that the type can be negated.

This comment has been minimized.

@mcarton

mcarton Apr 17, 2016

Collaborator

This is limited to integers anyway

@mcarton

mcarton Apr 17, 2016

Collaborator

This is limited to integers anyway

This comment has been minimized.

@llogiq

llogiq Apr 17, 2016

Collaborator

Yeah, this text was from the workshop, when we didn't have the type test yet, and I forgot to change it.

We may want to extend to other known types where x * -1 == -x (i.e. f32, f64, perhaps some SIMD types?).

@llogiq

llogiq Apr 17, 2016

Collaborator

Yeah, this text was from the workshop, when we didn't have the type test yet, and I forgot to change it.

We may want to extend to other known types where x * -1 == -x (i.e. f32, f64, perhaps some SIMD types?).

@mcarton mcarton merged commit 0bc0670 into master Apr 17, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mcarton mcarton deleted the neg_multiply branch Apr 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment