Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Commit

Permalink
Merge pull request #705 from alexheretic/clippy-source
Browse files Browse the repository at this point in the history
Detect clippy diagnostic source
  • Loading branch information
Xanewok authored Feb 11, 2018
2 parents d8998b6 + 3aae14e commit 52f5348
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 26 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ rls-rustc = "0.2.1"
rls-span = { version = "0.4", features = ["serialize-serde"] }
rls-vfs = { version = "0.4", features = ["racer-impls"] }
rustfmt-nightly = { version = "0.3.8", optional = true }
clippy_lints = { version = "0.0.185", optional = true }
clippy_lints = { version = "0.0.186", optional = true }
serde = "1.0"
serde_json = "1.0"
serde_derive = "1.0"
Expand Down
103 changes: 81 additions & 22 deletions src/actions/post_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ fn parse_diagnostics(message: &str, group: u64) -> Option<FileDiagnostic> {
let rls_span = primary_span.rls_span().zero_indexed();
let suggestions = make_suggestions(&message.children, &rls_span.file);

let mut source = "rustc";
let diagnostic = {
let mut primary_message = diagnostic_msg.clone();
if let Some(ref primary_label) = primary_span.label {
Expand All @@ -224,14 +225,20 @@ fn parse_diagnostics(message: &str, group: u64) -> Option<FileDiagnostic> {
primary_message.push_str(&format!("\n\n{}", notes));
}

// A diagnostic source is quite likely to be clippy if it contains
// the further information link to the rust-clippy project.
if primary_message.contains("rust-clippy") {
source = "clippy"
}

Diagnostic {
range: ls_util::rls_to_range(rls_span.range),
severity: Some(severity(&message.level)),
code: Some(NumberOrString::String(match message.code {
Some(ref c) => c.code.clone(),
None => String::new(),
})),
source: Some("rustc".into()),
source: Some(source.to_owned()),
message: primary_message.trim().to_owned(),
group: if message.spans.iter().any(|x| !x.is_primary) {
Some(group)
Expand Down Expand Up @@ -268,7 +275,7 @@ fn parse_diagnostics(message: &str, group: u64) -> Option<FileDiagnostic> {
Some(ref c) => c.code.clone(),
None => String::new(),
})),
source: Some("rustc".into()),
source: Some(source.to_owned()),
message: secondary_message.trim().to_owned(),
group: Some(group),
}
Expand Down Expand Up @@ -376,12 +383,24 @@ impl IsWithin for DiagnosticSpan {
mod diagnostic_message_test {
use super::*;

/// Returns (primary message, secondary messages)
fn parsed_message(compiler_message: &str) -> (String, Vec<String>) {
fn parse_compiler_message(compiler_message: &str) -> FileDiagnostic {
let _ = ::env_logger::try_init();
let parsed = parse_diagnostics(compiler_message, 0)
.expect("failed to parse compiler message");
(parsed.diagnostic.message, parsed.secondaries.into_iter().map(|s| s.message).collect())
parse_diagnostics(compiler_message, 0)
.expect("failed to parse compiler message")
}

trait FileDiagnosticTestExt {
/// Returns (primary message, secondary messages)
fn to_messages(&self) -> (String, Vec<String>);
}

impl FileDiagnosticTestExt for FileDiagnostic {
fn to_messages(&self) -> (String, Vec<String>) {
(
self.diagnostic.message.clone(),
self.secondaries.iter().map(|d| d.message.clone()).collect()
)
}
}

/// ```
Expand All @@ -393,9 +412,16 @@ mod diagnostic_message_test {
/// ```
#[test]
fn message_use_after_move() {
let (msg, others) = parsed_message(
let diag = parse_compiler_message(
include_str!("../../test_data/compiler_message/use-after-move.json")
);

assert_eq!(diag.diagnostic.source, Some("rustc".into()));
for source in diag.secondaries.iter().map(|d| d.source.as_ref()) {
assert_eq!(source, Some(&"rustc".into()));
}

let (msg, others) = diag.to_messages();
assert_eq!(
msg,
"use of moved value: `s`\n\n\
Expand All @@ -416,9 +442,9 @@ mod diagnostic_message_test {
/// ```
#[test]
fn message_type_annotations_needed() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/type-annotations-needed.json")
);
).to_messages();
assert_eq!(
msg,
"type annotations needed\n\n\
Expand All @@ -438,9 +464,9 @@ mod diagnostic_message_test {
/// ```
#[test]
fn message_mismatched_types() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/mismatched-types.json")
);
).to_messages();
assert_eq!(
msg,
"mismatched types\n\n\
Expand All @@ -461,9 +487,9 @@ mod diagnostic_message_test {
/// ```
#[test]
fn message_not_mutable() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/not-mut.json")
);
).to_messages();
assert_eq!(
msg,
"cannot borrow immutable local variable `string` as mutable\n\n\
Expand All @@ -485,9 +511,9 @@ mod diagnostic_message_test {
/// ```
#[test]
fn message_consider_borrowing() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/consider-borrowing.json")
);
).to_messages();
assert_eq!(
msg,
r#"mismatched types
Expand All @@ -512,9 +538,9 @@ help: consider borrowing here: `&string`"#,
/// ```
#[test]
fn message_move_out_of_borrow() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/move-out-of-borrow.json")
);
).to_messages();
assert_eq!(msg, "cannot move out of borrowed content");

assert_eq!(others, vec!["hint: to prevent move, use `ref string` or `ref mut string`"]);
Expand All @@ -525,9 +551,9 @@ help: consider borrowing here: `&string`"#,
/// ```
#[test]
fn message_unused_use() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/unused-use.json")
);
).to_messages();
assert_eq!(msg, "unused import: `std::borrow::Cow`\n\n\
note: #[warn(unused_imports)] on by default");

Expand All @@ -536,12 +562,45 @@ help: consider borrowing here: `&string`"#,

#[test]
fn message_cannot_find_type() {
let (msg, others) = parsed_message(
let (msg, others) = parse_compiler_message(
include_str!("../../test_data/compiler_message/cannot-find-type.json")
);
).to_messages();
assert_eq!(msg, "cannot find type `HashSet` in this scope\n\n\
not found in this scope");

assert!(others.is_empty(), "{:?}", others);
}

/// ```
/// let _s = 1 / 1;
/// ```
#[test]
fn message_clippy_identity_op() {
let diag = parse_compiler_message(
include_str!("../../test_data/compiler_message/clippy-identity-op.json")
);

assert_eq!(diag.diagnostic.source, Some("clippy".into()));
for source in diag.secondaries.iter().map(|d| d.source.as_ref()) {
assert_eq!(source, Some(&"clippy".into()));
}

let (msg, others) = diag.to_messages();
println!("\n---message---\n{}\n---", msg);

let link = {
let link_index = msg.find("https://rust-lang-nursery.github.io/rust-clippy/")
.expect("no clippy link found in message");
&msg[link_index..]
};

assert_eq!(
msg,
"the operation is ineffective. Consider reducing it to `1`\n\n\
note: #[warn(identity_op)] implied by #[warn(clippy)]\n\
help: for further information visit ".to_owned() + link
);

assert!(others.is_empty(), "{:?}", others);
}
}
66 changes: 66 additions & 0 deletions test_data/compiler_message/clippy-identity-op.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{
"children": [{
"children": [],
"code": null,
"level": "note",
"message": "lint level defined here",
"rendered": null,
"spans": [{
"byte_end": 39,
"byte_start": 33,
"column_end": 15,
"column_start": 9,
"expansion": null,
"file_name": "src/main.rs",
"is_primary": true,
"label": null,
"line_end": 2,
"line_start": 2,
"suggested_replacement": null,
"text": [{
"highlight_end": 15,
"highlight_start": 9,
"text": "#![warn(clippy)]"
}]
}]
}, {
"children": [],
"code": null,
"level": "note",
"message": "#[warn(identity_op)] implied by #[warn(clippy)]",
"rendered": null,
"spans": []
}, {
"children": [],
"code": null,
"level": "help",
"message": "for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#identity_op",
"rendered": null,
"spans": []
}],
"code": {
"code": "identity_op",
"explanation": null
},
"level": "warning",
"message": "the operation is ineffective. Consider reducing it to `1`",
"rendered": "warning: the operation is ineffective. Consider reducing it to `1`\n --> src/main.rs:5:14\n |\n5 | let _s = 1 / 1;\n | ^^^^^\n |\nnote: lint level defined here\n --> src/main.rs:2:9\n |\n2 | #![warn(clippy)]\n | ^^^^^^\n = note: #[warn(identity_op)] implied by #[warn(clippy)]\n = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#identity_op\n\n",
"spans": [{
"byte_end": 73,
"byte_start": 68,
"column_end": 19,
"column_start": 14,
"expansion": null,
"file_name": "src/main.rs",
"is_primary": true,
"label": null,
"line_end": 5,
"line_start": 5,
"suggested_replacement": null,
"text": [{
"highlight_end": 19,
"highlight_start": 14,
"text": " let _s = 1 / 1;"
}]
}]
}

0 comments on commit 52f5348

Please sign in to comment.