Skip to content

Commit aa6a62a

Browse files
eliassjogreeneliassjogreen
authored
feat(linter): Implement rules for keeping a consistent spec comment style (#904)
1 parent 74c8927 commit aa6a62a

File tree

65 files changed

+544
-155
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+544
-155
lines changed

‎nova_lint/Cargo.toml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,22 @@ path = "ui/gc_scope_comes_last.rs"
2424
name = "gc_scope_is_only_passed_by_value"
2525
path = "ui/gc_scope_is_only_passed_by_value.rs"
2626

27+
[[example]]
28+
name = "no_it_performs_the_following"
29+
path = "ui/no_it_performs_the_following.rs"
30+
31+
[[example]]
32+
name = "no_multipage_spec"
33+
path = "ui/no_multipage_spec.rs"
34+
35+
[[example]]
36+
name = "spec_header_level"
37+
path = "ui/spec_header_level.rs"
38+
2739
[dependencies]
2840
clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "c936595d17413c1f08e162e117e504fb4ed126e4" }
2941
dylint_linting = { version = "5.0.0", features = ["constituent"] }
42+
regex = "1"
3043

3144
[dev-dependencies]
3245
dylint_testing = "5.0.0"

‎nova_lint/src/lib.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ extern crate rustc_trait_selection;
2525
mod agent_comes_first;
2626
mod gc_scope_comes_last;
2727
mod gc_scope_is_only_passed_by_value;
28+
mod no_it_performs_the_following;
29+
mod no_multipage_spec;
30+
mod spec_header_level;
2831
mod utils;
2932

3033
pub(crate) use utils::*;
@@ -34,6 +37,9 @@ pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint
3437
agent_comes_first::register_lints(sess, lint_store);
3538
gc_scope_comes_last::register_lints(sess, lint_store);
3639
gc_scope_is_only_passed_by_value::register_lints(sess, lint_store);
40+
no_it_performs_the_following::register_lints(sess, lint_store);
41+
no_multipage_spec::register_lints(sess, lint_store);
42+
spec_header_level::register_lints(sess, lint_store);
3743
}
3844

3945
#[test]

‎nova_lint/src/no_it_performs_the_following.rs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use std::sync::LazyLock;
2+
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use regex::{Regex, RegexBuilder};
5+
use rustc_ast::Attribute;
6+
use rustc_errors::Applicability;
7+
use rustc_lint::{EarlyContext, EarlyLintPass};
8+
use rustc_span::{BytePos, Span};
9+
10+
dylint_linting::declare_early_lint! {
11+
/// ### What it does
12+
///
13+
/// This lint disallows doc comments that contain the phrase "It performs the following steps when called".
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// This phrase is a leftover from copy-pasting the TC-39 specification and does not add value to the documentation.
18+
///
19+
/// ### Example
20+
///
21+
/// ```rust
22+
/// /// 7.1.2 ToBoolean ( argument )
23+
/// ///
24+
/// /// The abstract operation ToBoolean takes argument argument (an ECMAScript
25+
/// /// language value) and returns a Boolean. It converts argument to a value of
26+
/// /// type Boolean. It performs the following steps when called:
27+
/// fn to_boolean() {
28+
/// unimplemented!()
29+
/// }
30+
/// ```
31+
///
32+
/// Use instead:
33+
///
34+
/// ```rust
35+
/// /// 7.1.2 ToBoolean ( argument )
36+
/// ///
37+
/// /// The abstract operation ToBoolean takes argument argument (an ECMAScript
38+
/// /// language value) and returns a Boolean. It converts argument to a value of
39+
/// /// type Boolean.
40+
/// fn to_boolean() {
41+
/// unimplemented!()
42+
/// }
43+
/// ```
44+
pub NO_IT_PERFORMS_THE_FOLLOWING,
45+
Warn,
46+
"you should omit \"It performs the following steps when called\" from spec comments"
47+
}
48+
49+
impl EarlyLintPass for NoItPerformsTheFollowing {
50+
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
51+
static RE: LazyLock<Regex> = LazyLock::new(|| {
52+
RegexBuilder::new(r"It performs the following steps when called:?")
53+
.case_insensitive(true)
54+
.build()
55+
.unwrap()
56+
});
57+
58+
let Some(doc) = attr.doc_str().map(|sym| sym.to_string()) else {
59+
return;
60+
};
61+
let Some(matched) = RE.find(&doc) else { return };
62+
if matched.is_empty() {
63+
return;
64+
}
65+
66+
let span = Span::new(
67+
attr.span.lo() + BytePos(matched.start() as u32 + 3),
68+
attr.span.lo() + BytePos(matched.end() as u32 + 3),
69+
attr.span.ctxt(),
70+
attr.span.parent(),
71+
);
72+
73+
span_lint_and_then(
74+
cx,
75+
NO_IT_PERFORMS_THE_FOLLOWING,
76+
span,
77+
format!(
78+
"this comment contains \"{}\", a leftover from copy-pasting the TC-39 specification",
79+
matched.as_str()
80+
),
81+
|diag| {
82+
diag.span_suggestion_hidden(
83+
span,
84+
"consider removing this phrase",
85+
"",
86+
Applicability::MachineApplicable,
87+
);
88+
},
89+
);
90+
}
91+
}

‎nova_lint/src/no_multipage_spec.rs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
use std::sync::LazyLock;
2+
3+
use clippy_utils::diagnostics::span_lint_and_sugg;
4+
use regex::{Regex, RegexBuilder};
5+
use rustc_ast::Attribute;
6+
use rustc_errors::Applicability;
7+
use rustc_lint::{EarlyContext, EarlyLintPass};
8+
use rustc_span::{BytePos, Span};
9+
10+
dylint_linting::declare_early_lint! {
11+
/// ### What it does
12+
///
13+
/// This lint disallows linking to the multi-page TC-39 specification in documentation comments.
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// For nova the general practice is to link to the single-page version of the TC-39 specification.
18+
///
19+
/// ### Example
20+
///
21+
/// ```rust
22+
/// /// [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-toboolean)
23+
/// fn to_boolean() {
24+
/// unimplemented!()
25+
/// }
26+
/// ```
27+
///
28+
/// Use instead:
29+
///
30+
/// ```rust
31+
/// /// [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
32+
/// fn to_boolean() {
33+
/// unimplemented!()
34+
/// }
35+
/// ```
36+
pub NO_MULTIPAGE_SPEC,
37+
Warn,
38+
"you should link to the single-page spec instead of the multi-page spec"
39+
}
40+
41+
impl EarlyLintPass for NoMultipageSpec {
42+
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
43+
static RE: LazyLock<Regex> = LazyLock::new(|| {
44+
RegexBuilder::new(r"https?://tc39.es/ecma262/multipage/?[^#]*")
45+
.build()
46+
.unwrap()
47+
});
48+
49+
let Some(doc) = attr.doc_str().map(|sym| sym.to_string()) else {
50+
return;
51+
};
52+
let Some(matched) = RE.find(&doc) else { return };
53+
if matched.is_empty() {
54+
return;
55+
}
56+
57+
let span = Span::new(
58+
attr.span.lo() + BytePos(matched.start() as u32 + 3),
59+
attr.span.lo() + BytePos(matched.end() as u32 + 3),
60+
attr.span.ctxt(),
61+
attr.span.parent(),
62+
);
63+
64+
span_lint_and_sugg(
65+
cx,
66+
NO_MULTIPAGE_SPEC,
67+
span,
68+
"linking to the multi-page TC-39 specification",
69+
"use the single-page version instead",
70+
"https://tc39.es/ecma262/".to_owned(),
71+
Applicability::MachineApplicable,
72+
);
73+
}
74+
}

‎nova_lint/src/spec_header_level.rs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use std::sync::LazyLock;
2+
3+
use clippy_utils::diagnostics::span_lint_and_sugg;
4+
use regex::{Regex, RegexBuilder};
5+
use rustc_ast::Attribute;
6+
use rustc_errors::Applicability;
7+
use rustc_lint::{EarlyContext, EarlyLintPass};
8+
use rustc_span::{BytePos, Span};
9+
10+
dylint_linting::declare_early_lint! {
11+
/// ### What it does
12+
///
13+
/// This lint checks that the header level of your documentation comments
14+
/// matches the header level of the TC-39 specification, with a max cap of
15+
/// 3 levels.
16+
///
17+
/// ### Why is this bad?
18+
///
19+
/// The TC-39 specification uses a specific header level for each section,
20+
/// and to be consistent with the spec and in our codebase, your
21+
/// documentation comments should match that level.
22+
///
23+
/// ### Example
24+
///
25+
/// ```rust
26+
/// /// ## [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
27+
/// fn to_boolean() {
28+
/// unimplemented!()
29+
/// }
30+
/// ```
31+
///
32+
/// Use instead:
33+
///
34+
/// ```rust
35+
/// /// ### [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
36+
/// fn to_boolean() {
37+
/// unimplemented!()
38+
/// }
39+
/// ```
40+
pub SPEC_HEADER_LEVEL,
41+
Warn,
42+
"you should match the header level of the TC-39 spec in your documentation comments"
43+
}
44+
45+
impl EarlyLintPass for SpecHeaderLevel {
46+
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
47+
static RE: LazyLock<Regex> = LazyLock::new(|| {
48+
RegexBuilder::new(r"^(\s*)(#*)\s\[([0-9B.]*)[^]]*]\(https?://tc39\.es/ecma262/.*\)")
49+
.build()
50+
.unwrap()
51+
});
52+
53+
let Some(doc) = attr.doc_str().map(|sym| sym.to_string()) else {
54+
return;
55+
};
56+
let Some(captures) = RE.captures(&doc) else {
57+
return;
58+
};
59+
60+
let spaces = captures
61+
.get(1)
62+
.map(|spaces| spaces.len() as u32)
63+
.unwrap_or(0);
64+
let header_level = captures
65+
.get(2)
66+
.map(|hashes| hashes.len() as u32)
67+
.unwrap_or(0);
68+
let expected_level = captures
69+
.get(3)
70+
.map(|numbering| numbering.as_str().chars().filter(|&c| c == '.').count() as u32 + 1)
71+
.unwrap_or(0)
72+
.min(3);
73+
74+
if header_level == expected_level {
75+
return;
76+
}
77+
78+
let span = Span::new(
79+
attr.span.lo() + BytePos(3 + spaces),
80+
attr.span.lo() + BytePos(3 + spaces + header_level),
81+
attr.span.ctxt(),
82+
attr.span.parent(),
83+
);
84+
85+
span_lint_and_sugg(
86+
cx,
87+
SPEC_HEADER_LEVEL,
88+
span,
89+
"the header level of your comment and the TC-39 specification does not match",
90+
"use the correct header level",
91+
"#".repeat(expected_level as usize),
92+
Applicability::MachineApplicable,
93+
);
94+
}
95+
}

‎nova_lint/ui/no_it_performs_the_following.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#![allow(dead_code, unused_variables)]
2+
3+
/// 7.1.2 ToBoolean ( argument )
4+
///
5+
/// The abstract operation ToBoolean takes argument argument (an ECMAScript
6+
/// language value) and returns a Boolean. It converts argument to a value of
7+
/// type Boolean. It performs the following steps when called:
8+
fn to_boolean() {
9+
unimplemented!()
10+
}
11+
12+
fn main() {
13+
unimplemented!()
14+
}

‎nova_lint/ui/no_it_performs_the_following.stderr‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
warning: this comment contains "It performs the following steps when called:", a leftover from copy-pasting the TC-39 specification
2+
--> $DIR/no_it_performs_the_following.rs:7:19
3+
|
4+
LL | /// type Boolean. It performs the following steps when called:
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(no_it_performs_the_following)]` on by default
8+
= help: consider removing this phrase
9+
10+
warning: 1 warning emitted
11+

‎nova_lint/ui/no_multipage_spec.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#![allow(dead_code, unused_variables)]
2+
3+
/// ### [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-toboolean)
4+
fn to_boolean() {
5+
unimplemented!()
6+
}
7+
8+
fn main() {
9+
unimplemented!()
10+
}

‎nova_lint/ui/no_multipage_spec.stderr‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
warning: linking to the multi-page TC-39 specification
2+
--> $DIR/no_multipage_spec.rs:3:40
3+
|
4+
LL | /// ### [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-toboolean)
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the single-page version instead: `https://tc39.es/ecma262/`
6+
|
7+
= note: `#[warn(no_multipage_spec)]` on by default
8+
9+
warning: 1 warning emitted
10+

‎nova_lint/ui/spec_header_level.rs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#![allow(dead_code, unused_variables)]
2+
3+
/// [1 Scope](https://tc39.es/ecma262/#sec-scope)
4+
5+
enum Instruction {
6+
/// Performs steps 3 and 4 from the [UnaryExpression - Runtime Semantics](https://tc39.es/ecma262/#sec-unary-minus-operator-runtime-semantics-evaluation).
7+
UnaryMinus,
8+
}
9+
10+
/// [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
11+
fn to_boolean_nothing() {
12+
unimplemented!()
13+
}
14+
15+
/// # [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
16+
fn to_boolean_wrong_1() {
17+
unimplemented!()
18+
}
19+
20+
/// #### [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
21+
fn to_boolean_wrong_2() {
22+
unimplemented!()
23+
}
24+
25+
/// ### [B.2.1.1 escape ( string )](https://tc39.es/ecma262/#sec-escape-string)
26+
fn escape() {
27+
unimplemented!()
28+
}
29+
30+
/// ### [7.1.2 ToBoolean ( argument )](https://tc39.es/ecma262/#sec-toboolean)
31+
fn to_boolean() {
32+
unimplemented!()
33+
}
34+
35+
fn main() {
36+
unimplemented!()
37+
}

0 commit comments

Comments
 (0)