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.

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

Change default level of INVALID_HTML_TAGS to warning and stabilize it #101720

Merged
merged 7 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ struct HandlerInner {
/// would be unnecessary repetition.
taught_diagnostics: FxHashSet<DiagnosticId>,

/// Used to suggest rustc --explain <error code>
/// Used to suggest rustc --explain `<error code>`
emitted_diagnostic_codes: FxIndexSet<DiagnosticId>,

/// This set contains a hash of every diagnostic that has been emitted by
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,10 +707,12 @@ pub(super) fn check_opaque_for_cycles<'tcx>(
/// check those cases in the `param_env` of that function, which may have
/// bounds not on this opaque type:
///
/// type X<T> = impl Clone
/// ```ignore (illustrative)
/// type X<T> = impl Clone;
/// fn f<T: Clone>(t: T) -> X<T> {
/// t
/// }
/// ```
///
/// Without this check the above code is incorrectly accepted: we would ICE if
/// some tried, for example, to clone an `Option<X<&mut ()>>`.
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_hir_analysis/src/constrained_generic_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ pub fn identify_constrained_generic_params<'tcx>(
/// ```
/// The impl's predicates are collected from left to right. Ignoring
/// the implicit `Sized` bounds, these are
/// * T: Debug
/// * U: Iterator
/// * <U as Iterator>::Item = T -- a desugared ProjectionPredicate
/// * `T: Debug`
/// * `U: Iterator`
/// * `<U as Iterator>::Item = T` -- a desugared ProjectionPredicate
///
/// When we, for example, try to go over the trait-reference
/// `IntoIter<u32> as Trait`, we substitute the impl parameters with fresh
Expand All @@ -132,12 +132,16 @@ pub fn identify_constrained_generic_params<'tcx>(
///
/// We *do* have to be somewhat careful when projection targets contain
/// projections themselves, for example in
///
/// ```ignore (illustrative)
/// impl<S,U,V,W> Trait for U where
/// /* 0 */ S: Iterator<Item = U>,
/// /* - */ U: Iterator,
/// /* 1 */ <U as Iterator>::Item: ToOwned<Owned=(W,<V as Iterator>::Item)>
/// /* 2 */ W: Iterator<Item = V>
/// /* 3 */ V: Debug
/// ```
///
/// we have to evaluate the projections in the order I wrote them:
/// `V: Debug` requires `V` to be evaluated. The only projection that
/// *determines* `V` is 2 (1 contains it, but *does not determine it*,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,10 @@ fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node
///
/// Example
///
/// ```ignore (illustrative)
/// impl<A, B> Foo<A> for B { /* impl2 */ }
/// impl<C> Foo<Vec<C>> for C { /* impl1 */ }
/// ```
///
/// Would return `S1 = [C]` and `S2 = [Vec<C>, C]`.
fn get_impl_substs<'tcx>(
Expand Down Expand Up @@ -225,13 +227,17 @@ fn unconstrained_parent_impl_substs<'tcx>(
///
/// For example forbid the following:
///
/// ```ignore (illustrative)
/// impl<A> Tr for A { }
/// impl<B> Tr for (B, B) { }
/// ```
///
/// Note that only consider the unconstrained parameters of the base impl:
///
/// ```ignore (illustrative)
/// impl<S, I: IntoIterator<Item = S>> Tr<S> for I { }
/// impl<T> Tr<T> for Vec<T> { }
/// ```
///
/// The substs for the parent impl here are `[T, Vec<T>]`, which repeats `T`,
/// but `S` is constrained in the parent impl, so `parent_substs` is only
Expand All @@ -256,8 +262,10 @@ fn check_duplicate_params<'tcx>(
///
/// For example forbid the following:
///
/// ```ignore (illustrative)
/// impl<A> Tr for A { }
/// impl Tr for &'static i32 { }
/// ```
fn check_static_lifetimes<'tcx>(
tcx: TyCtxt<'tcx>,
parent_substs: &Vec<GenericArg<'tcx>>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> {
/// modes #42640) may look like `Some(x)` but in fact have
/// implicit deref patterns attached (e.g., it is really
/// `&Some(x)`). In that case, we return the "outermost" type
/// (e.g., `&Option<T>).
/// (e.g., `&Option<T>`).
pub(crate) fn pat_ty_adjusted(&self, pat: &hir::Pat<'_>) -> McResult<Ty<'tcx>> {
// Check for implicit `&` types wrapping the pattern; note
// that these are never attached to binding patterns, so
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ pub enum StmtKind<'tcx> {
/// `let pat: ty = <INIT>`
initializer: Option<ExprId>,

/// `let pat: ty = <INIT> else { <ELSE> }
/// `let pat: ty = <INIT> else { <ELSE> }`
else_block: Option<BlockId>,

/// The lint level for this `let` statement.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,13 @@ impl<'tcx> AdtDef<'tcx> {
self.flags().contains(AdtFlags::IS_PHANTOM_DATA)
}

/// Returns `true` if this is Box<T>.
/// Returns `true` if this is `Box<T>`.
#[inline]
pub fn is_box(self) -> bool {
self.flags().contains(AdtFlags::IS_BOX)
}

/// Returns `true` if this is UnsafeCell<T>.
/// Returns `true` if this is `UnsafeCell<T>`.
#[inline]
pub fn is_unsafe_cell(self) -> bool {
self.flags().contains(AdtFlags::IS_UNSAFE_CELL)
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_parse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ mod errors;
// uses a HOF to parse anything, and <source> includes file and
// `source_str`.

/// A variant of 'panictry!' that works on a Vec<Diagnostic> instead of a single DiagnosticBuilder.
/// A variant of 'panictry!' that works on a `Vec<Diagnostic>` instead of a single
/// `DiagnosticBuilder`.
macro_rules! panictry_buffer {
($handler:expr, $e:expr) => {{
use rustc_errors::FatalError;
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_trait_selection/src/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ impl<'tcx> AutoTraitFinder<'tcx> {
/// struct Foo<T> { data: Box<T> }
/// ```
///
/// then this might return that Foo<T>: Send if T: Send (encoded in the AutoTraitResult type).
/// The analysis attempts to account for custom impls as well as other complex cases. This
/// result is intended for use by rustdoc and other such consumers.
/// then this might return that `Foo<T>: Send` if `T: Send` (encoded in the AutoTraitResult
/// type). The analysis attempts to account for custom impls as well as other complex cases.
/// This result is intended for use by rustdoc and other such consumers.
///
/// (Note that due to the coinductive nature of Send, the full and correct result is actually
/// quite simple to generate. That is, when a type has no custom impl, it is Send iff its field
/// types are all Send. So, in our example, we might have that Foo<T>: Send if Box<T>: Send.
/// types are all Send. So, in our example, we might have that `Foo<T>: Send` if `Box<T>: Send`.
/// But this is often not the best way to present to the user.)
///
/// Warning: The API should be considered highly unstable, and it may be refactored or removed
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ enum ProjectionCandidate<'tcx> {
/// From a where-clause in the env or object type
ParamEnv(ty::PolyProjectionPredicate<'tcx>),

/// From the definition of `Trait` when you have something like <<A as Trait>::B as Trait2>::C
/// From the definition of `Trait` when you have something like
/// `<<A as Trait>::B as Trait2>::C`.
TraitDef(ty::PolyProjectionPredicate<'tcx>),

/// Bounds specified on an object type
Expand Down Expand Up @@ -1367,7 +1368,7 @@ fn assemble_candidates_from_param_env<'cx, 'tcx>(
);
}

/// In the case of a nested projection like <<A as Foo>::FooT as Bar>::BarT, we may find
/// In the case of a nested projection like `<<A as Foo>::FooT as Bar>::BarT`, we may find
/// that the definition of `Foo` has some clues:
///
/// ```ignore (illustrative)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
/// must be met of course). One obvious case this comes up is
/// marker traits like `Send`. Think of a linked list:
///
/// struct List<T> { data: T, next: Option<Box<List<T>>> }
/// struct List<T> { data: T, next: Option<Box<List<T>>> }
///
/// `Box<List<T>>` will be `Send` if `T` is `Send` and
/// `Option<Box<List<T>>>` is `Send`, and in turn
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ declare_rustdoc_lint! {
///
/// [rustdoc book]: ../../../rustdoc/lints.html#invalid_html_tags
INVALID_HTML_TAGS,
Allow,
Warn,
"detects invalid HTML tags in doc comments"
}

Expand Down
6 changes: 2 additions & 4 deletions src/librustdoc/passes/html_tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ struct InvalidHtmlTagsLinter<'a, 'tcx> {
}

pub(crate) fn check_invalid_html_tags(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
if cx.tcx.sess.is_nightly_build() {
let mut coll = InvalidHtmlTagsLinter { cx };
coll.visit_crate(&krate);
}
let mut coll = InvalidHtmlTagsLinter { cx };
coll.visit_crate(&krate);
krate
}

Expand Down
9 changes: 9 additions & 0 deletions src/test/rustdoc-ui/intra-doc/malformed-generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,26 @@
//! [Vec<] //~ ERROR
//! [Vec<Box<T] //~ ERROR
//! [Vec<Box<T>] //~ ERROR
//~^ WARN
//! [Vec<Box<T>>>] //~ ERROR
//~^ WARN
//! [Vec<T>>>] //~ ERROR
//~^ WARN
//! [<Vec] //~ ERROR
//! [Vec::<] //~ ERROR
//! [<T>] //~ ERROR
//~^ WARN
//! [<invalid syntax>] //~ ERROR
//~^ WARN
//! [Vec:<T>:new()] //~ ERROR
//~^ WARN
//! [Vec<<T>>] //~ ERROR
//~^ WARN
//! [Vec<>] //~ ERROR
//! [Vec<<>>] //~ ERROR

// FIXME(#74563) support UFCS
//! [<Vec as IntoIterator>::into_iter] //~ ERROR
//~^ WARN
//! [<Vec<T> as IntoIterator>::iter] //~ ERROR
//~^ WARN
82 changes: 69 additions & 13 deletions src/test/rustdoc-ui/intra-doc/malformed-generics.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,80 +23,136 @@ LL | //! [Vec<Box<T>]
| ^^^^^^^^^^ unbalanced angle brackets

error: unresolved link to `Vec<Box<T>>>`
--> $DIR/malformed-generics.rs:6:6
--> $DIR/malformed-generics.rs:7:6
|
LL | //! [Vec<Box<T>>>]
| ^^^^^^^^^^^^ unbalanced angle brackets

error: unresolved link to `Vec<T>>>`
--> $DIR/malformed-generics.rs:7:6
--> $DIR/malformed-generics.rs:9:6
|
LL | //! [Vec<T>>>]
| ^^^^^^^^ unbalanced angle brackets

error: unresolved link to `<Vec`
--> $DIR/malformed-generics.rs:8:6
--> $DIR/malformed-generics.rs:11:6
|
LL | //! [<Vec]
| ^^^^ unbalanced angle brackets

error: unresolved link to `Vec::<`
--> $DIR/malformed-generics.rs:9:6
--> $DIR/malformed-generics.rs:12:6
|
LL | //! [Vec::<]
| ^^^^^^ unbalanced angle brackets

error: unresolved link to `<T>`
--> $DIR/malformed-generics.rs:10:6
--> $DIR/malformed-generics.rs:13:6
|
LL | //! [<T>]
| ^^^ missing type for generic parameters

error: unresolved link to `<invalid syntax>`
--> $DIR/malformed-generics.rs:11:6
--> $DIR/malformed-generics.rs:15:6
|
LL | //! [<invalid syntax>]
| ^^^^^^^^^^^^^^^^ missing type for generic parameters

error: unresolved link to `Vec:<T>:new`
--> $DIR/malformed-generics.rs:12:6
--> $DIR/malformed-generics.rs:17:6
|
LL | //! [Vec:<T>:new()]
| ^^^^^^^^^^^^^ has invalid path separator

error: unresolved link to `Vec<<T>>`
--> $DIR/malformed-generics.rs:13:6
--> $DIR/malformed-generics.rs:19:6
|
LL | //! [Vec<<T>>]
| ^^^^^^^^ too many angle brackets

error: unresolved link to `Vec<>`
--> $DIR/malformed-generics.rs:14:6
--> $DIR/malformed-generics.rs:21:6
|
LL | //! [Vec<>]
| ^^^^^ empty angle brackets

error: unresolved link to `Vec<<>>`
--> $DIR/malformed-generics.rs:15:6
--> $DIR/malformed-generics.rs:22:6
|
LL | //! [Vec<<>>]
| ^^^^^^^ too many angle brackets

error: unresolved link to `<Vec as IntoIterator>::into_iter`
--> $DIR/malformed-generics.rs:18:6
--> $DIR/malformed-generics.rs:25:6
|
LL | //! [<Vec as IntoIterator>::into_iter]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ fully-qualified syntax is unsupported
|
= note: see https://github.com/rust-lang/rust/issues/74563 for more information

error: unresolved link to `<Vec<T> as IntoIterator>::iter`
--> $DIR/malformed-generics.rs:19:6
--> $DIR/malformed-generics.rs:27:6
|
LL | //! [<Vec<T> as IntoIterator>::iter]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ fully-qualified syntax is unsupported
|
= note: see https://github.com/rust-lang/rust/issues/74563 for more information

error: aborting due to 15 previous errors
warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:5:13
|
LL | //! [Vec<Box<T>]
| ^^^
|
= note: `#[warn(rustdoc::invalid_html_tags)]` on by default
Copy link
Member

@camelid camelid Sep 14, 2022

Choose a reason for hiding this comment

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

@rfcbot concern intra-doc link suggestion

Hmm, this should be suggesting wrapping the contents of the link with ``. I've seen people writing links like [Vec<T>] before, so we should definitely be giving them a solution to the lint, which is also a better way to write their code.

Copy link
Contributor

@notriddle notriddle Sep 14, 2022

Choose a reason for hiding this comment

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

I added a test case for [Vec<T>], for completeness' sake, but it already worked.

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:74:14
|
LL | /// This [Vec<i32>] thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This [`Vec<i32>`] thing!
| + +

It's not suggesting anything here because of the mismatched angle brackets.


warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:7:13
|
LL | //! [Vec<Box<T>>>]
| ^^^

warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:9:9
|
LL | //! [Vec<T>>>]
| ^^^

warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:13:6
|
LL | //! [<T>]
| ^^^

warning: unclosed HTML tag `invalid`
--> $DIR/malformed-generics.rs:15:6
|
LL | //! [<invalid syntax>]
| ^^^^^^^^

warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:17:10
|
LL | //! [Vec:<T>:new()]
| ^^^

warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:19:10
|
LL | //! [Vec<<T>>]
| ^^^

warning: unclosed HTML tag `Vec`
--> $DIR/malformed-generics.rs:25:6
|
LL | //! [<Vec as IntoIterator>::into_iter]
| ^^^^

warning: unclosed HTML tag `T`
--> $DIR/malformed-generics.rs:27:10
|
LL | //! [<Vec<T> as IntoIterator>::iter]
| ^^^

error: aborting due to 15 previous errors; 9 warnings emitted

Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@
//! [`empty`]

// @has - '//a[@href="../empty2/index.html"]' 'empty2'
//! [empty2<x>]
//! [`empty2<x>`]
Loading