Skip to content

Commit

Permalink
Rollup merge of #108760 - clubby789:autolintstuff, r=wesleywiser
Browse files Browse the repository at this point in the history
Add lint to deny diagnostics composed of static strings

r? ghost

I'm hoping to have a lint that semi-automatically converts simple diagnostics such as `struct_span_err(span, "msg").help("msg").span_note(span2, "msg").emit()` to typed session diagnostics. It's quite hacky and not entirely working because of problems with `x fix` but should hopefully help reduce some of the work.
I'm going to start trying to apply what I can from this, but opening this as a draft in case anyone wants to develop on it.

cc #100717
  • Loading branch information
matthiaskrgr authored Apr 26, 2023
2 parents 8763965 + 0138513 commit 309496c
Show file tree
Hide file tree
Showing 37 changed files with 578 additions and 201 deletions.
19 changes: 19 additions & 0 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,25 @@ builtin_macros_format_pos_mismatch = {$n} positional {$n ->
[one] argument
*[more] arguments
} in format string, but {$desc}
builtin_macros_offset_of_expected_field = expected field
builtin_macros_offset_of_expected_two_args = expected 2 arguments
builtin_macros_test_case_non_item = `#[test_case]` attribute is only allowed on items
builtin_macros_test_bad_fn = {$kind} functions cannot be used for tests
.label = `{$kind}` because of this
builtin_macros_asm_explicit_register_name = explicit register arguments cannot have names
builtin_macros_asm_mutually_exclusive = the `{$opt1}` and `{$opt2}` options are mutually exclusive
builtin_macros_asm_pure_combine = the `pure` option must be combined with either `nomem` or `readonly`
builtin_macros_asm_pure_no_output = asm with the `pure` option must have at least one output
builtin_macros_asm_modifier_invalid = asm template modifier must be a single character
builtin_macros_test_runner_invalid = `test_runner` argument must be a path
builtin_macros_test_runner_nargs = `#![test_runner(..)]` accepts exactly 1 argument
28 changes: 8 additions & 20 deletions compiler/rustc_builtin_macros/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use rustc_span::{InnerSpan, Span};
use rustc_target::asm::InlineAsmArch;
use smallvec::smallvec;

use crate::errors;

pub struct AsmArgs {
pub templates: Vec<P<ast::Expr>>,
pub operands: Vec<(ast::InlineAsmOperand, Span)>,
Expand Down Expand Up @@ -205,7 +207,7 @@ pub fn parse_asm_args<'a>(
// of the argument available.
if explicit_reg {
if name.is_some() {
diag.struct_span_err(span, "explicit register arguments cannot have names").emit();
diag.emit_err(errors::AsmExplicitRegisterName { span });
}
args.reg_args.insert(slot);
} else if let Some(name) = name {
Expand Down Expand Up @@ -240,25 +242,19 @@ pub fn parse_asm_args<'a>(
&& args.options.contains(ast::InlineAsmOptions::READONLY)
{
let spans = args.options_spans.clone();
diag.struct_span_err(spans, "the `nomem` and `readonly` options are mutually exclusive")
.emit();
diag.emit_err(errors::AsmMutuallyExclusive { spans, opt1: "nomem", opt2: "readonly" });
}
if args.options.contains(ast::InlineAsmOptions::PURE)
&& args.options.contains(ast::InlineAsmOptions::NORETURN)
{
let spans = args.options_spans.clone();
diag.struct_span_err(spans, "the `pure` and `noreturn` options are mutually exclusive")
.emit();
diag.emit_err(errors::AsmMutuallyExclusive { spans, opt1: "pure", opt2: "noreturn" });
}
if args.options.contains(ast::InlineAsmOptions::PURE)
&& !args.options.intersects(ast::InlineAsmOptions::NOMEM | ast::InlineAsmOptions::READONLY)
{
let spans = args.options_spans.clone();
diag.struct_span_err(
spans,
"the `pure` option must be combined with either `nomem` or `readonly`",
)
.emit();
diag.emit_err(errors::AsmPureCombine { spans });
}

let mut have_real_output = false;
Expand All @@ -285,11 +281,7 @@ pub fn parse_asm_args<'a>(
}
}
if args.options.contains(ast::InlineAsmOptions::PURE) && !have_real_output {
diag.struct_span_err(
args.options_spans.clone(),
"asm with the `pure` option must have at least one output",
)
.emit();
diag.emit_err(errors::AsmPureNoOutput { spans: args.options_spans.clone() });
}
if args.options.contains(ast::InlineAsmOptions::NORETURN) && !outputs_sp.is_empty() {
let err = diag
Expand Down Expand Up @@ -705,11 +697,7 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
.ty_span
.map(|sp| template_sp.from_inner(InnerSpan::new(sp.start, sp.end)))
.unwrap_or(template_sp);
ecx.struct_span_err(
span,
"asm template modifier must be a single character",
)
.emit();
ecx.emit_err(errors::AsmModifierInvalid { span });
modifier = None;
}

Expand Down
68 changes: 68 additions & 0 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,3 +551,71 @@ pub(crate) struct FormatPositionalMismatch {
#[subdiagnostic]
pub(crate) highlight: SingleLabelManySpans,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_test_case_non_item)]
pub(crate) struct TestCaseNonItem {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_test_bad_fn)]
pub(crate) struct TestBadFn {
#[primary_span]
pub(crate) span: Span,
#[label]
pub(crate) cause: Span,
pub(crate) kind: &'static str,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_asm_explicit_register_name)]
pub(crate) struct AsmExplicitRegisterName {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_asm_mutually_exclusive)]
pub(crate) struct AsmMutuallyExclusive {
#[primary_span]
pub(crate) spans: Vec<Span>,
pub(crate) opt1: &'static str,
pub(crate) opt2: &'static str,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_asm_pure_combine)]
pub(crate) struct AsmPureCombine {
#[primary_span]
pub(crate) spans: Vec<Span>,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_asm_pure_no_output)]
pub(crate) struct AsmPureNoOutput {
#[primary_span]
pub(crate) spans: Vec<Span>,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_asm_modifier_invalid)]
pub(crate) struct AsmModifierInvalid {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_test_runner_invalid)]
pub(crate) struct TestRunnerInvalid {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_test_runner_nargs)]
pub(crate) struct TestRunnerNargs {
#[primary_span]
pub(crate) span: Span,
}
16 changes: 4 additions & 12 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::errors;
/// The expansion from a test function to the appropriate test struct for libtest
/// Ideally, this code would be in libtest but for efficiency and error messages it lives here.
use crate::util::{check_builtin_macro_attribute, warn_on_duplicate_attribute};
Expand Down Expand Up @@ -40,12 +41,7 @@ pub fn expand_test_case(
unreachable!()
},
_ => {
ecx.struct_span_err(
anno_item.span(),
"`#[test_case]` attribute is only allowed on items",
)
.emit();

ecx.emit_err(errors::TestCaseNonItem { span: anno_item.span() });
return vec![];
}
};
Expand Down Expand Up @@ -533,15 +529,11 @@ fn has_test_signature(cx: &ExtCtxt<'_>, i: &ast::Item) -> bool {
match &i.kind {
ast::ItemKind::Fn(box ast::Fn { sig, generics, .. }) => {
if let ast::Unsafe::Yes(span) = sig.header.unsafety {
sd.struct_span_err(i.span, "unsafe functions cannot be used for tests")
.span_label(span, "`unsafe` because of this")
.emit();
sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" });
return false;
}
if let ast::Async::Yes { span, .. } = sig.header.asyncness {
sd.struct_span_err(i.span, "async functions cannot be used for tests")
.span_label(span, "`async` because of this")
.emit();
sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "async" });
return false;
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use tracing::debug;

use std::{iter, mem};

use crate::errors;

#[derive(Clone)]
struct Test {
span: Span,
Expand Down Expand Up @@ -385,11 +387,11 @@ fn get_test_runner(sd: &rustc_errors::Handler, krate: &ast::Crate) -> Option<ast
[single] => match single.meta_item() {
Some(meta_item) if meta_item.is_word() => return Some(meta_item.path.clone()),
_ => {
sd.struct_span_err(span, "`test_runner` argument must be a path").emit();
sd.emit_err(errors::TestRunnerInvalid { span });
}
},
_ => {
sd.struct_span_err(span, "`#![test_runner(..)]` accepts exactly 1 argument").emit();
sd.emit_err(errors::TestRunnerNargs { span });
}
}
None
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,16 @@ codegen_ssa_invalid_monomorphization_unsupported_cast = invalid monomorphization
codegen_ssa_invalid_monomorphization_unsupported_operation = invalid monomorphization of `{$name}` intrinsic: unsupported operation on `{$in_ty}` with element `{$in_elem}`
codegen_ssa_invalid_monomorphization_expected_vector_element_type = invalid monomorphization of `{$name}` intrinsic: expected element type `{$expected_element}` of vector type `{$vector_type}` to be a signed or unsigned integer type
codegen_ssa_invalid_no_sanitize = invalid argument for `no_sanitize`
.note = expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`
codegen_ssa_invalid_link_ordinal_nargs = incorrect number of arguments to `#[link_ordinal]`
.note = the attribute requires exactly one argument
codegen_ssa_illegal_link_ordinal_format = illegal ordinal format in `link_ordinal`
.note = an unsuffixed integer value, e.g., `1`, is expected
codegen_ssa_target_feature_safe_trait = `#[target_feature(..)]` cannot be applied to safe trait method
.label = cannot be applied to safe trait method
.label_def = not an `unsafe` function
16 changes: 4 additions & 12 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc_span::symbol::Ident;
use rustc_span::{sym, Span};
use rustc_target::spec::{abi, SanitizerSet};

use crate::errors;
use crate::target_features::from_target_feature;
use crate::{errors::ExpectedUsedSymbol, target_features::check_target_feature_trait_unsafe};

Expand Down Expand Up @@ -334,10 +335,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS
}
_ => {
tcx.sess
.struct_span_err(item.span(), "invalid argument for `no_sanitize`")
.note("expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`")
.emit();
tcx.sess.emit_err(errors::InvalidNoSanitize { span: item.span() });
}
}
}
Expand Down Expand Up @@ -608,10 +606,7 @@ fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
let sole_meta_list = match meta_item_list {
Some([item]) => item.lit(),
Some(_) => {
tcx.sess
.struct_span_err(attr.span, "incorrect number of arguments to `#[link_ordinal]`")
.note("the attribute requires exactly one argument")
.emit();
tcx.sess.emit_err(errors::InvalidLinkOrdinalNargs { span: attr.span });
return None;
}
_ => None,
Expand Down Expand Up @@ -642,10 +637,7 @@ fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
None
}
} else {
tcx.sess
.struct_span_err(attr.span, "illegal ordinal format in `link_ordinal`")
.note("an unsuffixed integer value, e.g., `1`, is expected")
.emit();
tcx.sess.emit_err(errors::InvalidLinkOrdinalFormat { span: attr.span });
None
}
}
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,3 +981,37 @@ impl IntoDiagnosticArg for ExpectedPointerMutability {
}
}
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_invalid_no_sanitize)]
#[note]
pub struct InvalidNoSanitize {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_invalid_link_ordinal_nargs)]
#[note]
pub struct InvalidLinkOrdinalNargs {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_illegal_link_ordinal_format)]
#[note]
pub struct InvalidLinkOrdinalFormat {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_target_feature_safe_trait)]
pub struct TargetFeatureSafeTrait {
#[primary_span]
#[label]
pub span: Span,
#[label(codegen_ssa_label_def)]
pub def: Span,
}
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::errors;
use rustc_ast::ast;
use rustc_attr::InstructionSetAttr;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -443,14 +444,10 @@ pub fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, attr_s
if let DefKind::AssocFn = tcx.def_kind(id) {
let parent_id = tcx.local_parent(id);
if let DefKind::Trait | DefKind::Impl { of_trait: true } = tcx.def_kind(parent_id) {
tcx.sess
.struct_span_err(
attr_span,
"`#[target_feature(..)]` cannot be applied to safe trait method",
)
.span_label(attr_span, "cannot be applied to safe trait method")
.span_label(tcx.def_span(id), "not an `unsafe` function")
.emit();
tcx.sess.emit_err(errors::TargetFeatureSafeTrait {
span: attr_span,
def: tcx.def_span(id),
});
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_expand/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,7 @@ expand_proc_macro_panicked =
expand_proc_macro_derive_tokens =
proc-macro derive produced unparsable tokens
expand_duplicate_matcher_binding = duplicate matcher binding
.label = duplicate binding
.label2 = previous binding
10 changes: 10 additions & 0 deletions compiler/rustc_expand/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,3 +397,13 @@ pub struct ProcMacroDeriveTokens {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(expand_duplicate_matcher_binding)]
pub struct DuplicateMatcherBinding {
#[primary_span]
#[label]
pub span: Span,
#[label(expand_label2)]
pub prev: Span,
}
6 changes: 2 additions & 4 deletions compiler/rustc_expand/src/mbe/macro_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
//! Kleene operators under which a meta-variable is repeating is the concatenation of the stacks
//! stored when entering a macro definition starting from the state in which the meta-variable is
//! bound.
use crate::errors;
use crate::mbe::{KleeneToken, TokenTree};

use rustc_ast::token::{Delimiter, Token, TokenKind};
Expand Down Expand Up @@ -281,10 +282,7 @@ fn check_binders(
// Duplicate binders at the top-level macro definition are errors. The lint is only
// for nested macro definitions.
sess.span_diagnostic
.struct_span_err(span, "duplicate matcher binding")
.span_label(span, "duplicate binding")
.span_label(prev_info.span, "previous binding")
.emit();
.emit_err(errors::DuplicateMatcherBinding { span, prev: prev_info.span });
*valid = false;
} else {
binders.insert(name, BinderInfo { span, ops: ops.into() });
Expand Down
Loading

0 comments on commit 309496c

Please sign in to comment.