Skip to content

UI test cleanup: Extract lint from methods.rs test #3610

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 5, 2019
Merged

UI test cleanup: Extract lint from methods.rs test #3610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaaee23
UI test cleanup: Extract lint from methods.rs test
phansch Jan 2, 2019
0c54913
Extract IteratorFalsePositives into option_helpers.rs
phansch Jan 2, 2019
c84a894
rustfmt
phansch Jan 2, 2019
a5d3f37
Use compiletest's aux-build header instead of include macro
phansch Jan 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions tests/ui/auxiliary/option_helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#![allow(dead_code, unused_variables)]

/// Utility macro to test linting behavior in `option_methods()`
/// The lints included in `option_methods()` should not lint if the call to map is partially
/// within a macro
#[macro_export]
macro_rules! opt_map {
($opt:expr, $map:expr) => {
($opt).map($map)
};
}

/// Struct to generate false positive for Iterator-based lints
#[derive(Copy, Clone)]
pub struct IteratorFalsePositives {
pub foo: u32,
}

impl IteratorFalsePositives {
pub fn filter(self) -> IteratorFalsePositives {
self
}

pub fn next(self) -> IteratorFalsePositives {
self
}

pub fn find(self) -> Option<u32> {
Some(self.foo)
}

pub fn position(self) -> Option<u32> {
Some(self.foo)
}

pub fn rposition(self) -> Option<u32> {
Some(self.foo)
}

pub fn nth(self, n: usize) -> Option<u32> {
Some(self.foo)
}

pub fn skip(self, _: usize) -> IteratorFalsePositives {
self
}
}
38 changes: 4 additions & 34 deletions tests/ui/iter_skip_next.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,44 +7,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:option_helpers.rs

#![warn(clippy::iter_skip_next)]
#![allow(clippy::blacklisted_name)]

/// Struct to generate false positive for Iterator-based lints
#[derive(Copy, Clone)]
struct IteratorFalsePositives {
foo: u32,
}

impl IteratorFalsePositives {
fn filter(self) -> IteratorFalsePositives {
self
}

fn next(self) -> IteratorFalsePositives {
self
}

fn find(self) -> Option<u32> {
Some(self.foo)
}
extern crate option_helpers;

fn position(self) -> Option<u32> {
Some(self.foo)
}

fn rposition(self) -> Option<u32> {
Some(self.foo)
}

fn nth(self, n: usize) -> Option<u32> {
Some(self.foo)
}

fn skip(self, _: usize) -> IteratorFalsePositives {
self
}
}
use option_helpers::IteratorFalsePositives;

/// Checks implementation of `ITER_SKIP_NEXT` lint
fn iter_skip_next() {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/iter_skip_next.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
--> $DIR/iter_skip_next.rs:52:13
--> $DIR/iter_skip_next.rs:22:13
|
LL | let _ = some_vec.iter().skip(42).next();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::iter-skip-next` implied by `-D warnings`

error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
--> $DIR/iter_skip_next.rs:53:13
--> $DIR/iter_skip_next.rs:23:13
|
LL | let _ = some_vec.iter().cycle().skip(42).next();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
--> $DIR/iter_skip_next.rs:54:13
--> $DIR/iter_skip_next.rs:24:13
|
LL | let _ = (1..10).skip(10).next();
| ^^^^^^^^^^^^^^^^^^^^^^^

error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
--> $DIR/iter_skip_next.rs:55:14
--> $DIR/iter_skip_next.rs:25:14
|
LL | let _ = &some_vec[..].iter().skip(3).next();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
73 changes: 7 additions & 66 deletions tests/ui/methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:option_helpers.rs

#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)]
#![allow(
clippy::blacklisted_name,
Expand All @@ -22,6 +24,9 @@
clippy::useless_format
)]

#[macro_use]
extern crate option_helpers;
Copy link
Member

Choose a reason for hiding this comment

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

Is this extern crate necessary? Aren't we using the 2018 edition in the tests? Or do we need this because of the aux-build of compiletest-rs?

use option_helpers::opt_map;

should be possible.

Copy link
Member Author

@phansch phansch Jan 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Currently it's necessary yes. I think it's an issue with compiletest: Manishearth/compiletest-rs#150


use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
Expand All @@ -31,6 +36,8 @@ use std::iter::FromIterator;
use std::rc::{self, Rc};
use std::sync::{self, Arc};

use option_helpers::IteratorFalsePositives;

pub struct T;

impl T {
Expand Down Expand Up @@ -101,13 +108,6 @@ impl Mul<T> for T {
fn mul(self, other: T) -> T { self } // no error, obviously
}

/// Utility macro to test linting behavior in `option_methods()`
/// The lints included in `option_methods()` should not lint if the call to map is partially
/// within a macro
macro_rules! opt_map {
($opt:expr, $map:expr) => {($opt).map($map)};
}

/// Checks implementation of the following lints:
/// * `OPTION_MAP_UNWRAP_OR`
/// * `OPTION_MAP_UNWRAP_OR_ELSE`
Expand Down Expand Up @@ -169,29 +169,6 @@ fn option_methods() {
);
}

/// Checks implementation of the following lints:
/// * `RESULT_MAP_UNWRAP_OR_ELSE`
fn result_methods() {
let res: Result<i32, ()> = Ok(1);

// Check RESULT_MAP_UNWRAP_OR_ELSE
// single line case
let _ = res.map(|x| x + 1)

.unwrap_or_else(|e| 0); // should lint even though this call is on a separate line
// multi line cases
let _ = res.map(|x| {
x + 1
}
).unwrap_or_else(|e| 0);
let _ = res.map(|x| x + 1)
.unwrap_or_else(|e|
0
);
// macro case
let _ = opt_map!(res, |x| x + 1).unwrap_or_else(|e| 0); // should not lint
}

/// Struct to generate false positives for things with .iter()
#[derive(Copy, Clone)]
struct HasIter;
Expand All @@ -206,42 +183,6 @@ impl HasIter {
}
}

/// Struct to generate false positive for Iterator-based lints
#[derive(Copy, Clone)]
struct IteratorFalsePositives {
foo: u32,
}

impl IteratorFalsePositives {
fn filter(self) -> IteratorFalsePositives {
self
}

fn next(self) -> IteratorFalsePositives {
self
}

fn find(self) -> Option<u32> {
Some(self.foo)
}

fn position(self) -> Option<u32> {
Some(self.foo)
}

fn rposition(self) -> Option<u32> {
Some(self.foo)
}

fn nth(self, n: usize) -> Option<u32> {
Some(self.foo)
}

fn skip(self, _: usize) -> IteratorFalsePositives {
self
}
}

/// Checks implementation of `FILTER_NEXT` lint
fn filter_next() {
let v = vec![3, 2, 1, 0, -1, -2, -3];
Expand Down
Loading