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.

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

Jump to bottom

[beta] Do not accidentally treat multi-segment meta-items as single-segment #59259

Merged
merged 2 commits into from Mar 26, 2019
Merged

[beta] Do not accidentally treat multi-segment meta-items as single-segment #59259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdd934e
Do not accidentally treat multi-segment meta-items as single-segment
petrochenkov Feb 28, 2019
f8b17c5
Restore APIs used by clippy
petrochenkov Mar 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/librustc/hir/check_attr.rs
View file
Open in desktop
Expand Up @@ -166,7 +166,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
// ```
let hints: Vec<_> = item.attrs
.iter()
.filter(|attr| attr.name() == "repr")
.filter(|attr| attr.check_name("repr"))
.filter_map(|attr| attr.meta_item_list())
.flatten()
.collect();
Expand All @@ -177,15 +177,15 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
let mut is_transparent = false;

for hint in &hints {
let name = if let Some(name) = hint.name() {
let name = if let Some(name) = hint.ident_str() {
name
} else {
// Invalid repr hint like repr(42). We don't check for unrecognized hints here
// (libsyntax does that), so just ignore it.
continue;
};

let (article, allowed_targets) = match &*name.as_str() {
let (article, allowed_targets) = match name {
"C" | "align" => {
is_c |= name == "C";
if target != Target::Struct &&
Expand Down Expand Up @@ -313,7 +313,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {

fn check_used(&self, item: &hir::Item, target: Target) {
for attr in &item.attrs {
if attr.name() == "used" && target != Target::Static {
if attr.check_name("used") && target != Target::Static {
self.tcx.sess
.span_err(attr.span, "attribute must be applied to a `static` variable");
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ich/impls_syntax.rs
View file
Open in desktop
Expand Up @@ -197,7 +197,8 @@ impl<'a> HashStable<StableHashingContext<'a>> for [ast::Attribute] {
let filtered: SmallVec<[&ast::Attribute; 8]> = self
.iter()
.filter(|attr| {
!attr.is_sugared_doc && !hcx.is_ignored_attr(attr.name())
!attr.is_sugared_doc &&
!attr.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name))
})
.collect();

Expand All @@ -224,7 +225,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for ast::Attribute {
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
// Make sure that these have been filtered out.
debug_assert!(!hcx.is_ignored_attr(self.name()));
debug_assert!(!self.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name)));
debug_assert!(!self.is_sugared_doc);

let ast::Attribute {
Expand Down
21 changes: 11 additions & 10 deletions src/librustc/lint/levels.rs
View file
Open in desktop
Expand Up @@ -193,7 +193,7 @@ impl<'a> LintLevelsBuilder<'a> {
struct_span_err!(sess, span, E0452, "malformed lint attribute")
};
for attr in attrs {
let level = match Level::from_str(&attr.name().as_str()) {
let level = match attr.ident_str().and_then(|name| Level::from_str(name)) {
None => continue,
Some(lvl) => lvl,
};
Expand Down Expand Up @@ -254,9 +254,9 @@ impl<'a> LintLevelsBuilder<'a> {
}

for li in metas {
let word = match li.word() {
Some(word) => word,
None => {
let meta_item = match li.meta_item() {
Some(meta_item) if meta_item.is_word() => meta_item,
_ => {
let mut err = bad_attr(li.span);
if let Some(item) = li.meta_item() {
if let ast::MetaItemKind::NameValue(_) = item.node {
Expand All @@ -269,23 +269,24 @@ impl<'a> LintLevelsBuilder<'a> {
continue;
}
};
let tool_name = if let Some(lint_tool) = word.is_scoped() {
if !attr::is_known_lint_tool(lint_tool) {
let tool_name = if meta_item.ident.segments.len() > 1 {
let tool_ident = meta_item.ident.segments[0].ident;
if !attr::is_known_lint_tool(tool_ident) {
span_err!(
sess,
lint_tool.span,
tool_ident.span,
E0710,
"an unknown tool name found in scoped lint: `{}`",
word.ident
meta_item.ident
);
continue;
}

Some(lint_tool.as_str())
Some(tool_ident.as_str())
} else {
None
};
let name = word.name();
let name = meta_item.ident.segments.last().expect("empty lint name").ident.name;
match store.check_lint_name(&name.as_str(), tool_name) {
CheckLintNameResult::Ok(ids) => {
let src = LintSource::Node(name, li.span, reason);
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/lib_features.rs
View file
Open in desktop
Expand Up @@ -63,9 +63,9 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> {
for meta in metas {
if let Some(mi) = meta.meta_item() {
// Find the `feature = ".."` meta-item.
match (&*mi.name().as_str(), mi.value_str()) {
("feature", val) => feature = val,
("since", val) => since = val,
match (mi.ident_str(), mi.value_str()) {
(Some("feature"), val) => feature = val,
(Some("since"), val) => since = val,
_ => {}
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/librustc/middle/stability.rs
View file
Open in desktop
Expand Up @@ -197,11 +197,12 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
} else {
// Emit errors for non-staged-api crates.
for attr in attrs {
let tag = attr.name();
if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" {
attr::mark_used(attr);
self.tcx.sess.span_err(attr.span(), "stability attributes may not be used \
outside of the standard library");
if let Some(tag) = attr.ident_str() {
if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" {
attr::mark_used(attr);
self.tcx.sess.span_err(attr.span, "stability attributes may not be used \
outside of the standard library");
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc/session/config.rs
View file
Open in desktop
Expand Up @@ -1810,7 +1810,8 @@ pub fn parse_cfgspecs(cfgspecs: Vec<String>) -> ast::CrateConfig {
error!("argument value must be a string");
}
MetaItemKind::NameValue(..) | MetaItemKind::Word => {
return (meta_item.name(), meta_item.value_str());
let ident = meta_item.ident().expect("multi-segment cfg key");
return (ident.name, meta_item.value_str());
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/librustc/traits/on_unimplemented.rs
View file
Open in desktop
Expand Up @@ -177,10 +177,12 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
for command in self.subcommands.iter().chain(Some(self)).rev() {
if let Some(ref condition) = command.condition {
if !attr::eval_condition(condition, &tcx.sess.parse_sess, &mut |c| {
options.contains(&(
c.name().as_str().to_string(),
c.value_str().map(|s| s.as_str().to_string())
))
c.ident_str().map_or(false, |name| {
options.contains(&(
name.to_string(),
c.value_str().map(|s| s.as_str().to_string())
))
})
}) {
debug!("evaluate: skipping {:?} due to condition", command);
continue
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_incremental/assert_dep_graph.rs
View file
Open in desktop
Expand Up @@ -99,9 +99,9 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
fn argument(&self, attr: &ast::Attribute) -> Option<ast::Name> {
let mut value = None;
for list_item in attr.meta_item_list().unwrap_or_default() {
match list_item.word() {
Some(word) if value.is_none() =>
value = Some(word.name()),
match list_item.ident() {
Some(ident) if list_item.is_word() && value.is_none() =>
value = Some(ident.name),
_ =>
// FIXME better-encapsulate meta_item (don't directly access `node`)
span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item.node),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_incremental/persist/dirty_clean.rs
View file
Open in desktop
Expand Up @@ -576,7 +576,7 @@ fn expect_associated_value(tcx: TyCtxt<'_, '_, '_>, item: &NestedMetaItem) -> as
if let Some(value) = item.value_str() {
value
} else {
let msg = if let Some(name) = item.name() {
let msg = if let Some(name) = item.ident_str() {
format!("associated value expected for `{}`", name)
} else {
"expected an associated value".to_string()
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/builtin.rs
View file
Open in desktop
Expand Up @@ -757,7 +757,7 @@ impl LintPass for DeprecatedAttr {
impl EarlyLintPass for DeprecatedAttr {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) {
for &&(n, _, _, ref g) in &self.depr_attrs {
if attr.name() == n {
if attr.ident_str() == Some(n) {
if let &AttributeGate::Gated(Stability::Deprecated(link, suggestion),
ref name,
ref reason,
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_lint/unused.rs
View file
Open in desktop
Expand Up @@ -265,19 +265,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes {
}
}

let name = attr.name();
let name = attr.ident_str();
if !attr::is_used(attr) {
debug!("Emitting warning for: {:?}", attr);
cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute");
// Is it a builtin attribute that must be used at the crate level?
let known_crate = BUILTIN_ATTRIBUTES.iter()
.find(|&&(builtin, ty, ..)| name == builtin && ty == AttributeType::CrateLevel)
.find(|&&(builtin, ty, ..)| {
name == Some(builtin) && ty == AttributeType::CrateLevel
})
.is_some();

// Has a plugin registered this attribute as one that must be used at
// the crate level?
let plugin_crate = plugin_attributes.iter()
.find(|&&(ref x, t)| name == &**x && AttributeType::CrateLevel == t)
.find(|&&(ref x, t)| name == Some(x) && AttributeType::CrateLevel == t)
.is_some();
if known_crate || plugin_crate {
let msg = match attr.style {
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_passes/layout_test.rs
View file
Open in desktop
Expand Up @@ -53,9 +53,7 @@ impl<'a, 'tcx> VarianceTest<'a, 'tcx> {
// The `..` are the names of fields to dump.
let meta_items = attr.meta_item_list().unwrap_or_default();
for meta_item in meta_items {
let name = meta_item.word().map(|mi| mi.name().as_str());
let name = name.as_ref().map(|s| &s[..]).unwrap_or("");

let name = meta_item.ident_str().unwrap_or("");
match name {
"abi" => {
self.tcx
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_plugin/load.rs
View file
Open in desktop
Expand Up @@ -56,10 +56,10 @@ pub fn load_plugins(sess: &Session,

for plugin in plugins {
// plugins must have a name and can't be key = value
match plugin.name() {
match plugin.ident_str() {
Some(name) if !plugin.is_value_str() => {
let args = plugin.meta_item_list().map(ToOwned::to_owned);
loader.load_plugin(plugin.span, &name.as_str(), args.unwrap_or_default());
loader.load_plugin(plugin.span, name, args.unwrap_or_default());
},
_ => call_malformed_plugin_attribute(sess, attr.span),
}
Expand Down
23 changes: 11 additions & 12 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Open in desktop
Expand Up @@ -466,10 +466,9 @@ impl<'a> Resolver<'a> {
if let Some(attr) = attr::find_by_name(&item.attrs, "proc_macro_derive") {
if let Some(trait_attr) =
attr.meta_item_list().and_then(|list| list.get(0).cloned()) {
if let Some(ident) = trait_attr.name().map(Ident::with_empty_ctxt) {
let sp = trait_attr.span;
if let Some(ident) = trait_attr.ident() {
let def = Def::Macro(def.def_id(), MacroKind::ProcMacroStub);
self.define(parent, ident, MacroNS, (def, vis, sp, expansion));
self.define(parent, ident, MacroNS, (def, vis, ident.span, expansion));
}
}
}
Expand Down Expand Up @@ -815,9 +814,9 @@ impl<'a> Resolver<'a> {
break;
}
MetaItemKind::List(nested_metas) => for nested_meta in nested_metas {
match nested_meta.word() {
Some(word) => single_imports.push((word.name(), word.span)),
None => ill_formed(nested_meta.span),
match nested_meta.ident() {
Some(ident) if nested_meta.is_word() => single_imports.push(ident),
Copy link
Contributor

Choose a reason for hiding this comment

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

For a follow up PR, I see this pattern in multiple places, it'd be nice to add a method get_word_ident() to nested_meta so you can use if let everywhere the alternative is discarded/an error.

_ => ill_formed(nested_meta.span),
}
}
MetaItemKind::NameValue(..) => ill_formed(meta.span),
Expand Down Expand Up @@ -853,23 +852,23 @@ impl<'a> Resolver<'a> {
self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
});
} else {
for (name, span) in single_imports.iter().cloned() {
let ident = Ident::with_empty_ctxt(name);
for ident in single_imports.iter().cloned() {
let result = self.resolve_ident_in_module(
ModuleOrUniformRoot::Module(module),
ident,
MacroNS,
None,
false,
span,
ident.span,
);
if let Ok(binding) = result {
let directive = macro_use_directive(span);
let directive = macro_use_directive(ident.span);
self.potentially_unused_imports.push(directive);
let imported_binding = self.import(binding, directive);
self.legacy_import_macro(name, imported_binding, span, allow_shadowing);
self.legacy_import_macro(ident.name, imported_binding,
ident.span, allow_shadowing);
} else {
span_err!(self.session, span, E0469, "imported macro not found");
span_err!(self.session, ident.span, E0469, "imported macro not found");
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/librustdoc/clean/cfg.rs
View file
Open in desktop
Expand Up @@ -58,7 +58,13 @@ impl Cfg {
/// If the content is not properly formatted, it will return an error indicating what and where
/// the error is.
pub fn parse(cfg: &MetaItem) -> Result<Cfg, InvalidCfgError> {
let name = cfg.name();
let name = match cfg.ident() {
Some(ident) => ident.name,
None => return Err(InvalidCfgError {
msg: "expected a single identifier",
span: cfg.span
}),
};
match cfg.node {
MetaItemKind::Word => Ok(Cfg::Cfg(name, None)),
MetaItemKind::NameValue(ref lit) => match lit.node {
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Expand Up @@ -491,7 +491,7 @@ impl Item {

pub fn is_non_exhaustive(&self) -> bool {
self.attrs.other_attrs.iter()
.any(|a| a.name().as_str() == "non_exhaustive")
.any(|a| a.check_name("non_exhaustive"))
}

/// Returns a documentation-level item type from the item.
Expand Down Expand Up @@ -3665,7 +3665,7 @@ impl Clean<Vec<Item>> for doctree::ExternCrate {
fn clean(&self, cx: &DocContext<'_, '_, '_>) -> Vec<Item> {

let please_inline = self.vis.node.is_pub() && self.attrs.iter().any(|a| {
a.name() == "doc" && match a.meta_item_list() {
a.check_name("doc") && match a.meta_item_list() {
Some(l) => attr::list_contains_name(&l, "inline"),
None => false,
}
Expand Down Expand Up @@ -3704,7 +3704,7 @@ impl Clean<Vec<Item>> for doctree::Import {
// #[doc(no_inline)] attribute is present.
// Don't inline doc(hidden) imports so they can be stripped at a later stage.
let mut denied = !self.vis.node.is_pub() || self.attrs.iter().any(|a| {
a.name() == "doc" && match a.meta_item_list() {
a.check_name("doc") && match a.meta_item_list() {
Some(l) => attr::list_contains_name(&l, "no_inline") ||
attr::list_contains_name(&l, "hidden"),
None => false,
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/core.rs
View file
Open in desktop
Expand Up @@ -567,8 +567,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
for attr in krate.module.as_ref().unwrap().attrs.lists("doc") {
let diag = ctxt.sess().diagnostic();

let name = attr.name().map(|s| s.as_str());
let name = name.as_ref().map(|s| &s[..]);
let name = attr.ident_str();
if attr.is_word() {
if name == Some("no_default_passes") {
report_deprecated_attr("no_default_passes", diag);
Expand Down
14 changes: 6 additions & 8 deletions src/librustdoc/html/render.rs
View file
Open in desktop
Expand Up @@ -562,8 +562,7 @@ pub fn run(mut krate: clean::Crate,
// going to emit HTML
if let Some(attrs) = krate.module.as_ref().map(|m| &m.attrs) {
for attr in attrs.lists("doc") {
let name = attr.name().map(|s| s.as_str());
match (name.as_ref().map(|s| &s[..]), attr.value_str()) {
match (attr.ident_str(), attr.value_str()) {
(Some("html_favicon_url"), Some(s)) => {
scx.layout.favicon = s.to_string();
}
Expand Down Expand Up @@ -3718,19 +3717,19 @@ fn item_enum(w: &mut fmt::Formatter<'_>, cx: &Context, it: &clean::Item,
}

fn render_attribute(attr: &ast::MetaItem) -> Option<String> {
let name = attr.name();
let path = attr.ident.to_string();

if attr.is_word() {
Some(name.to_string())
Some(path)
} else if let Some(v) = attr.value_str() {
Some(format!("{} = {:?}", name, v.as_str()))
Some(format!("{} = {:?}", path, v.as_str()))
} else if let Some(values) = attr.meta_item_list() {
let display: Vec<_> = values.iter().filter_map(|attr| {
attr.meta_item().and_then(|mi| render_attribute(mi))
}).collect();

if display.len() > 0 {
Some(format!("{}({})", name, display.join(", ")))
Some(format!("{}({})", path, display.join(", ")))
} else {
None
}
Expand All @@ -3754,8 +3753,7 @@ fn render_attributes(w: &mut fmt::Formatter<'_>, it: &clean::Item) -> fmt::Resul
let mut attrs = String::new();

for attr in &it.attrs.other_attrs {
let name = attr.name();
if !ATTRIBUTE_WHITELIST.contains(&&*name.as_str()) {
if !attr.ident_str().map_or(false, |name| ATTRIBUTE_WHITELIST.contains(&name)) {
continue;
}
if let Some(s) = render_attribute(&attr.meta().unwrap()) {
Expand Down