rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.22k stars 1.51k forks source link

Lint suggestions based on MISRA 2004 #2227

Open xd009642 opened 6 years ago

xd009642 commented 6 years ago

As mentioned in #2215 I'm compiling a list of lints based on MISRA rules. For the C language MISRA 2004 defines C90 should be used so there are numerous rules forbidding certain C99 features, C++ features. And there are also rules written for machines which provide FPUs that don't use IEEE754 for floats. So I'm going to attempt to remove all the irrelevant rules and keep it to rules which are relevant to rust :smile:

So here it goes. All the relevant rules, a lot of these are probably already covered. And a number are actually already caught by the rust compiler!

Already solved by clippy

Already solved by the language

goodmanjonathan commented 6 years ago

I'd like to take some of these on! I already have a lint prepared for else if branches without an else.

shssoichiro commented 6 years ago

I think many more of these are solved by the language/compiler than are checked off. Someone with edit privileges may want to update the list.

goodmanjonathan commented 6 years ago

Yeah, I realized that too. We can definitely cross off the following:

oli-obk commented 6 years ago

Thanks. I only went through the list quickly. Feel free to post if I need to update any others.

pJunger commented 4 years ago

Maybe it would be better to analyze rulesets from other (more similar) languages as well (e.g. ADA or C++). In my opinion many of those rules do not make sense for Rust even if they are implementable (e.g. the single point of return).

C++:

Ada: ?

xd009642 commented 4 years ago

SPARK Ada is a "safer" subset designed for easier verification of safety critical systems

llogiq commented 4 years ago

While that's true, it still requires manual verification of the code, including setting up invariants, etc.

Are there no rule sets or lints for security relevant Ada code?

xd009642 commented 4 years ago

I didn't see any when I worked in aerospace. But then no new projects were done in Ada only legacy code used it. The lack of support for Ada in certain testing and analysis tools meant those parts of the code were painful as we had to manually verify :cry:

pJunger commented 4 years ago

@xd009642 I'm no expert on Ada, but wouldn't the Ravenscar profile be a better comparison?

I also found this wikipedia article on Coding conventions which lists 4 coding styles for Ada (including ones from NASA and ESA).

Additionally, I don't think that the availability of tooling (or lack thereof) is that relevant here - those rules still might be interesting for Rust (or clippy).

xd009642 commented 4 years ago

Additionally, I don't think that the availability of tooling (or lack thereof) is that relevant here

Well all the C/C++ standards we used had a lot of automated tooling and static analysis which automated the strenuous parts of reviewing. And I'm not familiar with Ravenscar, the legacy projects (systems on jets) I worked on used the SPARK subset or a standard written specifically for the project.

As well as looking at other coding standards could also peruse standards like DO-178B (aerospace), ISO 26262 (automotive) etc. to see what they assurances they need at the various safety levels

llogiq commented 4 years ago

Btw. re-reading the list above, the "no bitwise ops on signed ints" rule stood out, because it may be problematic in C, unlike in Rust. So we might want to have a fourth category "unneeded because of different semantics"...

pJunger commented 4 years ago

@xd009642 I know that Misra and other standards are (partly or mostly) checked by tools like QA-C or similar, but what I meant was: Just because these tools might not exist for Ada does not mean that one shouldn't have a look at those rules (because we might implement them for Rust).

I agree that safety standards for those various domains should be considered.

@llogiq That's why I mentioned those other coding standards - many of those rules definitely make sense for C, but not for Rust. It could provide a simple way to filter out C-specific rules.

synek317 commented 4 years ago

I don't understand most of the rules, could you please give me some more details?

Identifiers in different namespaces should not share the same name (with exception of struct elements)

how is this an issue in rust?

Identifier names should not be reused

so this would become a clippy warning?

fn foo(bar: impl AsRef<Path>) {
  let bar = bar.as_ref();
  // ...
}

static variables should only be referenced in the file they are defined in

Probably mutable static vars? Anyway, is this really an rust issue?

array size must be defined at compile time

Not rust related afaik?

a value of a "complex" expression of integer type may only be cast to a narrower type if it is the same signedness a value of a "complex" expression of float type may only be cast to a smaller float type

Can you give an example of 'comples' expression?

No dependency on operator precedence rules. Use brackets etc.

So this would trigger a warning? 2 + 5 * 10

&& and || can only be primary expressions a && b && c is good, a && (b && c) is bad do not mix logical and non-logical operators

How is a && (b && c) bad? Or how should I write a || (b && c)?

floating point values cannot be loop counters in loops

How this applies to rust?

no use of goto or continue

How is continue harmful? It sometimes improves readability or performance a lot.

a loop may not have more than one break statement a function shall have a single point of exit at the end of the function

Too general. In some cases, multiple returns or breaks improves readability

no recursion (directly or indirectly)

Why??? I know recursion may cause stack overflow easily, but come on. A lot of functions have nice recursive versions. Not every recursion is easy to change into iteration. Tail recursion may be optimized by the compiler.

only use pointer arithmetic on arrays

Is this applicable to rust?

imports should only be preceded by other imports

How can this affect the rust code negatively?

do not redefine things defined in the standard library

No more custom Result type? :(

dynamic memory allocation must not be used

So no more Vecs?

all if statements must have an else

Why? How?

pJunger commented 4 years ago

Does it really make sense to collect & discuss those rules here? In my opinion it would be great to have a separate repository for this (similar to the RFC repo?)

There a discussion could take place in order to find a set of rules that

Edit: Prototype can be found in this repo.

oli-obk commented 2 years ago

a very related analysis: https://github.com/PolySync/misra-rust/blob/master/MISRA-Rules.md