-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add lint for intra link resolution failure #51382
Changes from all commits
b000cf0
8c43c93
2e343f3
6d5e6b4
e6c7868
d2a4e42
231c61a
6a03884
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ pub use self::Visibility::{Public, Inherited}; | |
|
||
use syntax; | ||
use rustc_target::spec::abi::Abi; | ||
use syntax::ast::{self, AttrStyle, Ident}; | ||
use syntax::ast::{self, AttrStyle, NodeId, Ident}; | ||
use syntax::attr; | ||
use syntax::codemap::{dummy_spanned, Spanned}; | ||
use syntax::feature_gate::UnstableFeatures; | ||
|
@@ -46,9 +46,10 @@ use rustc::middle::stability; | |
use rustc::util::nodemap::{FxHashMap, FxHashSet}; | ||
use rustc_typeck::hir_ty_to_ty; | ||
use rustc::infer::region_constraints::{RegionConstraintData, Constraint}; | ||
use rustc::lint as lint; | ||
|
||
use std::collections::hash_map::Entry; | ||
use std::fmt; | ||
|
||
use std::default::Default; | ||
use std::{mem, slice, vec}; | ||
use std::iter::{FromIterator, once}; | ||
|
@@ -1283,10 +1284,16 @@ fn resolution_failure( | |
link_range.end + code_dox_len, | ||
); | ||
|
||
diag = cx.sess().struct_span_warn(sp, &msg); | ||
diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, | ||
NodeId::new(0), | ||
sp, | ||
&msg); | ||
diag.span_label(sp, "cannot be resolved, ignoring"); | ||
} else { | ||
diag = cx.sess().struct_span_warn(sp, &msg); | ||
diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, | ||
NodeId::new(0), | ||
sp, | ||
&msg); | ||
|
||
let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1); | ||
let line = dox[last_new_line_offset..].lines().next().unwrap_or(""); | ||
|
@@ -1303,8 +1310,13 @@ fn resolution_failure( | |
} | ||
diag | ||
} else { | ||
cx.sess().struct_span_warn(sp, &msg) | ||
cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, | ||
NodeId::new(0), | ||
sp, | ||
&msg) | ||
}; | ||
diag.help("to escape `[` and `]` characters, just add '\\' before them like \ | ||
`\\[` or `\\]`"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should better use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In many cases, it'll really be a link resolution failure. Not sure it'd be a nice idea to suggest that to remove the warning, they should not use auto-linking. :p |
||
diag.emit(); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ use rustc::middle::cstore::CrateStore; | |
use rustc::middle::privacy::AccessLevels; | ||
use rustc::ty::{self, TyCtxt, AllArenas}; | ||
use rustc::hir::map as hir_map; | ||
use rustc::lint; | ||
use rustc::lint::{self, LintPass}; | ||
use rustc::session::config::ErrorOutputType; | ||
use rustc::util::nodemap::{FxHashMap, FxHashSet}; | ||
use rustc_resolve as resolve; | ||
|
@@ -187,16 +187,33 @@ pub fn run_core(search_paths: SearchPaths, | |
_ => None | ||
}; | ||
|
||
let warning_lint = lint::builtin::WARNINGS.name_lower(); | ||
let intra_link_resolution_failure_name = lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE.name; | ||
let warnings_lint_name = lint::builtin::WARNINGS.name; | ||
let lints = lint::builtin::HardwiredLints.get_lints() | ||
.iter() | ||
.chain(rustc_lint::SoftLints.get_lints()) | ||
.filter_map(|lint| { | ||
if lint.name == warnings_lint_name || | ||
lint.name == intra_link_resolution_failure_name { | ||
None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really want to be excluding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the opposite, we're allowing everything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure that allowing a bunch of lints individually will act the same as allowing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we set all warnings at once using this one, then the intra-doc lint stop working. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohhhhh, that makes sense, i forgot about that. In that case, this is fine. |
||
} else { | ||
Some((lint.name_lower(), lint::Allow)) | ||
} | ||
}) | ||
.collect::<Vec<_>>(); | ||
|
||
let host_triple = TargetTriple::from_triple(config::host_triple()); | ||
// plays with error output here! | ||
let sessopts = config::Options { | ||
maybe_sysroot, | ||
search_paths, | ||
crate_types: vec![config::CrateTypeRlib], | ||
lint_opts: if !allow_warnings { vec![(warning_lint, lint::Allow)] } else { vec![] }, | ||
lint_cap: Some(lint::Allow), | ||
lint_opts: if !allow_warnings { | ||
lints | ||
} else { | ||
vec![] | ||
}, | ||
lint_cap: Some(lint::Forbid), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Observation: |
||
cg, | ||
externs, | ||
target_triple: triple.unwrap_or(host_triple), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
#![deny(intra_doc_link_resolution_failure)] | ||
|
||
/// [v2] //~ ERROR | ||
pub fn foo() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
error: `[v2]` cannot be resolved, ignoring it... | ||
--> $DIR/deny-intra-link-resolution-failure.rs:13:6 | ||
| | ||
13 | /// [v2] //~ ERROR | ||
| ^^ cannot be resolved, ignoring | ||
| | ||
note: lint level defined here | ||
--> $DIR/deny-intra-link-resolution-failure.rs:11:9 | ||
| | ||
11 | #![deny(intra_doc_link_resolution_failure)] | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really how it comes out when the error appears? Or is it an artifact of the UI testing? It would be really worrying if we were accidentally suggesting the wrong thing. EDIT: I mean it's showing a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @QuietMisdreavus Just an artifact of UI test normalization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Phew, i was getting nervous there. Imperio said he got the proper output when he ran the test locally, too, so i'll let this slide. |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did this list of lints come from? I see a similar list in
register_builtins
farther down, but this one seems to be missing some of those.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From this file. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, now i see it. Thanks for pointing that out.