Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positive in empty_line_after_outer_attr #2590

Merged
merged 3 commits into from
Mar 30, 2018

Conversation

phansch
Copy link
Member

@phansch phansch commented Mar 29, 2018

Before, when you had a block comment between an attribute and the
following item like this:

#[crate_type = "lib"]
/*

*/
pub struct Rust;

It would cause a false positive on the lint, because there is an empty
line inside the block comment.

This makes sure that basic block comments are detected and removed from
the snippet that was created before.

Fixes #2583

@CAD97
Copy link
Contributor

CAD97 commented Mar 30, 2018

I think a single line block comment will break this...?

let mut without = vec![];

// naive approach for block comments
let mut inside_comment = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be a counter, because in rust you can nest block comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 30, 2018

We should probably move this lint to the nursery group for now.

phansch added 3 commits March 30, 2018 12:36
Before, when you had a block comment between an attribute and the
following item like this:

```rust
\#[crate_type = "lib"]
/*

*/
pub struct Rust;
```

It would cause a false positive on the lint, because there is an empty
line inside the block comment.

This makes sure that basic block comments are detected and removed from
the snippet that was created before.
From the clippy side it's difficult to detect empty lines between
an attributes and the following item because empty lines and comments
are not part of the AST. The parsing currently works for basic cases
but is not perfect and can cause false positives.

Maybe libsyntax 2.0 will fix some of the problems around attributes but
comments will probably be never part of the AST so we would still have
to do some manual parsing.
@phansch phansch force-pushed the fix_another_false_positive branch from 38aaea9 to db1ec44 Compare March 30, 2018 10:48
@phansch
Copy link
Member Author

phansch commented Mar 30, 2018

I fully agree about moving it to the nursery 👍

@CAD97 I couldn't break with a single line block comment. I added an additional test for

#[crate_type = "lib"]
/* test */
pub struct T;

which doesn't break it. Did you have something else in mind?

@oli-obk oli-obk merged commit b7a0b97 into rust-lang:master Mar 30, 2018
@phansch phansch deleted the fix_another_false_positive branch March 30, 2018 11:15
@phansch phansch added the T-macros Type: Issues with macros and macro expansion label Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-macros Type: Issues with macros and macro expansion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants