linter-rust icon indicating copy to clipboard operation
linter-rust copied to clipboard

lint inside non-default cfg modules

Open alex opened this issue 10 years ago • 7 comments

As far as I can tell, modules which are not compiled by default, e.g.:

#[cfg(test)]
mod tests {
}

do not get linted

alex avatar Jul 20 '15 11:07 alex

+1 confirm, that would be very nice indeed.

colin-kiegel avatar Jul 20 '15 13:07 colin-kiegel

This is really easy to solve for code behind #[cfg(test)] and #[test], but the generic case of handling any arbitrary feature is much more difficult. Say you have code hidden behind a feature like #[cfg(feature = foo)]. This is toggled off by default. The only way to ensure that we lint every line of code would be to parse the Cargo.toml file and enable every feature we find in there when we run rustc or cargo. I guess it might work, but I don't know if enabling all features would always be a valid scenario. I see this being a problem if conflicting dependencies are used for different features. For instance, code behind a windows feature uses one version of a dependency and code for osx uses another. I don't know how often that sort of thing is done in rust. Either way, test code seems like the obvious low hanging fruit and that was easy.

psFried avatar Jul 26 '15 15:07 psFried

#[cfg(test)] satisfied my immediate use case, so I'm happy :-) Thanks! :sparkles:

alex avatar Jul 26 '15 15:07 alex

It's been a long while since this issue was originally created, so I wanted to give an update. Currently, it is possible to lint test code by selecting test from the Cargo Command drop down. The problem with this is that running cargo test takes a lot longer than cargo rustc with the no-trans option. Now that this PR has landed, it should now be possible to use cargo rustc and specify test or bench configurations. This should allow for much faster linting inside of test code.

Is everyone on board with changing the test cargo command to use cargo rustc with the no-trans option? Is anyone attached to the slower cargo test, which results in a finished test binary?

psFried avatar Mar 03 '16 23:03 psFried

no-trans sounds reasonable to me

colin-kiegel avatar Mar 03 '16 23:03 colin-kiegel

+1 for no-trans

White-Oak avatar Mar 04 '16 08:03 White-Oak

This is solved by #64, I guess.

White-Oak avatar Aug 17 '16 19:08 White-Oak