Skip to content

Hacky rustup #3905

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

Merged
merged 9 commits into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ rustc_tools_util = { version = "0.1.1", path = "rustc_tools_util"}
[dev-dependencies]
clippy_dev = { version = "0.0.1", path = "clippy_dev" }
cargo_metadata = "0.7.1"
compiletest_rs = "0.3.19"
compiletest_rs = { version = "=0.3.19", features = ["tmp", "stable"] }
libtest = "0.0.1"
lazy_static = "1.0"
serde_derive = "1.0"
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ impl LintPass for AttrPass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
if let Some(items) = &attr.meta_item_list() {
if let Some(ident) = attr.ident_str() {
match ident {
if let Some(ident) = attr.ident() {
match &*ident.as_str() {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
},
Expand Down Expand Up @@ -242,8 +242,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {

for attr in &item.attrs {
if let Some(lint_list) = &attr.meta_item_list() {
if let Some(ident) = attr.ident_str() {
match ident {
if let Some(ident) = attr.ident() {
match &*ident.as_str() {
"allow" | "warn" | "deny" | "forbid" => {
// whitelist `unused_imports` and `deprecated` for `use` items
// and `unused_imports` for `extern crate` items with `macro_use`
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,11 +516,11 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
for pat in &arm.pats {
if let PatKind::Path(ref path) = pat.deref().node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.did != p.def.def_id());
missing_variants.retain(|e| e.ctor_def_id != Some(p.def.def_id()));
}
} else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.did != p.def.def_id());
missing_variants.retain(|e| e.ctor_def_id != Some(p.def.def_id()));
}
}
}
Expand All @@ -539,7 +539,7 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
String::new()
};
// This path assumes that the enum type is imported into scope.
format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.did), suffix)
format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.def_id), suffix)
})
.collect();

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/missing_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ impl MissingDoc {
if let Some(meta) = meta;
if let MetaItemKind::List(list) = meta.node;
if let Some(meta) = list.get(0);
if let Some(name) = meta.ident_str();
if let Some(name) = meta.ident();
then {
name == "include"
name.as_str() == "include"
} else {
false
}
Expand Down
11 changes: 5 additions & 6 deletions clippy_lints/src/missing_inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline {
};

if let Some(trait_def_id) = trait_def_id {
if cx.tcx.hir().as_local_node_id(trait_def_id).is_some() {
if !cx.access_levels.is_exported(impl_item.hir_id) {
// If a trait is being implemented for an item, and the
// trait is not exported, we don't need #[inline]
return;
}
if cx.tcx.hir().as_local_node_id(trait_def_id).is_some() && !cx.access_levels.is_exported(impl_item.hir_id)
{
// If a trait is being implemented for an item, and the
// trait is not exported, we don't need #[inline]
return;
}
}

Expand Down
6 changes: 2 additions & 4 deletions clippy_lints/src/no_effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
if let ExprKind::Path(ref qpath) = callee.node {
let def = cx.tables.qpath_def(qpath, callee.hir_id);
match def {
Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => {
Def::Struct(..) | Def::Variant(..) | Def::Ctor(..) => {
!has_drop(cx, cx.tables.expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg))
},
_ => false,
Expand Down Expand Up @@ -166,9 +166,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec
if let ExprKind::Path(ref qpath) = callee.node {
let def = cx.tables.qpath_def(qpath, callee.hir_id);
match def {
Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..)
if !has_drop(cx, cx.tables.expr_ty(expr)) =>
{
Def::Struct(..) | Def::Variant(..) | Def::Ctor(..) if !has_drop(cx, cx.tables.expr_ty(expr)) => {
Some(args.iter().collect())
},
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl Pass {
},
ExprKind::Ret(Some(ref expr)) => Self::expression_returns_none(cx, expr),
ExprKind::Path(ref qp) => {
if let Def::VariantCtor(def_id, _) = cx.tables.qpath_def(qp, expression.hir_id) {
if let Def::Ctor(def_id, def::CtorOf::Variant, _) = cx.tables.qpath_def(qp, expression.hir_id) {
return match_def_path(cx.tcx, def_id, &OPTION_NONE);
}

Expand Down
14 changes: 10 additions & 4 deletions clippy_lints/src/ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,25 +157,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}) = higher::range(cx, expr);
if let Some(y) = y_plus_one(end);
then {
let span = expr.span
.ctxt()
.outer()
.expn_info()
.map(|info| info.call_site)
.unwrap_or(expr.span);
span_lint_and_then(
cx,
RANGE_PLUS_ONE,
expr.span,
span,
"an inclusive range would be more readable",
|db| {
let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
let end = Sugg::hir(cx, y, "y");
if let Some(is_wrapped) = &snippet_opt(cx, expr.span) {
if let Some(is_wrapped) = &snippet_opt(cx, span) {
if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
db.span_suggestion(
expr.span,
span,
"use",
format!("({}..={})", start, end),
Applicability::MaybeIncorrect,
);
} else {
db.span_suggestion(
expr.span,
span,
"use",
format!("{}..={}", start, end),
Applicability::MachineApplicable, // snippet
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
if path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfUpper.name() {
if self.item_path.def == path.def {
span_use_self_lint(self.cx, path);
} else if let Def::StructCtor(ctor_did, CtorKind::Fn) = path.def {
} else if let Def::Ctor(ctor_did, def::CtorOf::Struct, CtorKind::Fn) = path.def {
if self.item_path.def.opt_def_id() == self.cx.tcx.parent(ctor_did) {
span_use_self_lint(self.cx, path);
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool {
fn is_enum_variant(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> bool {
matches!(
cx.tables.qpath_def(qpath, id),
def::Def::Variant(..) | def::Def::VariantCtor(..)
def::Def::Variant(..) | def::Def::Ctor(_, def::CtorOf::Variant, _)
)
}

Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
then {
// report the error around the `vec!` not inside `<std macros>:`
let span = arg.span
.ctxt()
.outer()
.expn_info()
.map(|info| info.call_site)
.expect("unable to get call_site")
.ctxt()
.outer()
.expn_info()
Expand Down
46 changes: 37 additions & 9 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![feature(test)]

use compiletest_rs as compiletest;
extern crate test;
use libtest::TestDescAndFn;

use std::env::{set_var, var};
use std::ffi::OsStr;
Expand Down Expand Up @@ -74,15 +74,11 @@ fn run_mode(mode: &str, dir: PathBuf) {
compiletest::run_tests(&cfg);
}

fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<test::TestDescAndFn>) -> Result<bool, io::Error> {
fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<TestDescAndFn>) -> Result<bool, io::Error> {
let mut result = true;
let opts = compiletest::test_opts(config);
for dir in fs::read_dir(&config.src_base)? {
let dir = dir?;
if !dir.file_type()?.is_dir() {
continue;
}
let dir_path = dir.path();
let dir_path = dir.unwrap().path();
set_var("CARGO_MANIFEST_DIR", &dir_path);
for file in fs::read_dir(&dir_path)? {
let file = file?;
Expand All @@ -101,9 +97,25 @@ fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<test::TestDesc
let test_name = compiletest::make_test_name(&config, &paths);
let index = tests
.iter()
.position(|test| test.desc.name == test_name)
.position(|test| test.desc.name.to_string() == test_name.to_string())
.expect("The test should be in there");
result &= test::run_tests_console(&opts, vec![tests.swap_remove(index)])?;
let opts = libtest::TestOpts {
list: opts.list.clone(),
filter: opts.filter.clone(),
filter_exact: opts.filter_exact.clone(),
exclude_should_panic: Default::default(),
run_ignored: libtest::RunIgnored::No,
run_tests: opts.run_tests.clone(),
bench_benchmarks: opts.bench_benchmarks.clone(),
logfile: opts.logfile.clone(),
nocapture: opts.nocapture,
color: libtest::ColorConfig::AutoColor,
format: libtest::OutputFormat::Pretty,
test_threads: opts.test_threads,
skip: opts.skip.clone(),
options: libtest::Options::new(),
};
result &= libtest::run_tests_console(&opts, vec![tests.swap_remove(index)])?;
}
}
Ok(result)
Expand All @@ -114,6 +126,22 @@ fn run_ui_toml() {
let config = config("ui", path);
let tests = compiletest::make_tests(&config);

let tests = tests
.into_iter()
.map(|test| {
libtest::TestDescAndFn {
desc: libtest::TestDesc {
name: libtest::TestName::DynTestName(test.desc.name.to_string()),
ignore: test.desc.ignore,
allow_fail: test.desc.allow_fail,
should_panic: libtest::ShouldPanic::No,
},
// oli obk giving up
testfn: unsafe { std::mem::transmute(test.testfn) },
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put up an issue about this, there's probably someone who already knows how to write it without transmute right? 🤔

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 oli-obk meant opening issues only for broken tests, it's probably not possible to avoid transumte until libtest gets fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. It's just one line though, it doesn't seem that bad right?

}
})
.collect();

let res = run_ui_toml_tests(&config, tests);
match res {
Ok(true) => {},
Expand Down
26 changes: 13 additions & 13 deletions tests/ui/for_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -198,83 +198,83 @@ error: it is more concise to loop over references to containers instead of using
--> $DIR/for_loop.rs:170:15
|
LL | for _v in vec.iter() {}
| ^^^^^^^^^^ help: to write this more concisely, try: `&vec`
| ^^^^^^^^^^
|
= note: `-D clippy::explicit-iter-loop` implied by `-D warnings`

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:172:15
|
LL | for _v in vec.iter_mut() {}
| ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec`
| ^^^^^^^^^^^^^^

error: it is more concise to loop over containers instead of using explicit iteration methods`
--> $DIR/for_loop.rs:175:15
|
LL | for _v in out_vec.into_iter() {}
| ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec`
| ^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings`

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:178:15
|
LL | for _v in array.into_iter() {}
| ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array`
| ^^^^^^^^^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:183:15
|
LL | for _v in [1, 2, 3].iter() {}
| ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]`
| ^^^^^^^^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:187:15
|
LL | for _v in [0; 32].iter() {}
| ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]`
| ^^^^^^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:192:15
|
LL | for _v in ll.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&ll`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:195:15
|
LL | for _v in vd.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&vd`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:198:15
|
LL | for _v in bh.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&bh`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:201:15
|
LL | for _v in hm.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&hm`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:204:15
|
LL | for _v in bt.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&bt`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:207:15
|
LL | for _v in hs.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&hs`
| ^^^^^^^^^

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:210:15
|
LL | for _v in bs.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&bs`
| ^^^^^^^^^

error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
--> $DIR/for_loop.rs:212:15
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/into_iter_on_ref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn main() {
for _ in &[1, 2, 3] {}
for _ in vec![X, X] {}
for _ in &vec![X, X] {}
for _ in [1, 2, 3].iter() {} //~ ERROR equivalent to .iter()
for _ in [1, 2, 3].into_iter() {} //~ ERROR equivalent to .iter()

let _ = [1, 2, 3].iter(); //~ ERROR equivalent to .iter()
let _ = vec![1, 2, 3].into_iter();
Expand Down
14 changes: 4 additions & 10 deletions tests/ui/into_iter_on_ref.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:13:24
--> $DIR/into_iter_on_ref.rs:15:23
|
LL | for _ in [1, 2, 3].into_iter() {} //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
LL | let _ = [1, 2, 3].into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
|
note: lint level defined here
--> $DIR/into_iter_on_ref.rs:4:9
|
LL | #![deny(clippy::into_iter_on_array)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:15:23
|
LL | let _ = [1, 2, 3].into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`

error: this .into_iter() call is equivalent to .iter() and will not move the Vec
--> $DIR/into_iter_on_ref.rs:17:30
|
Expand Down Expand Up @@ -174,5 +168,5 @@ error: this .into_iter() call is equivalent to .iter() and will not move the Pat
LL | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`

error: aborting due to 28 previous errors
error: aborting due to 27 previous errors

Loading