Skip to content

Commit cbe77a2

Browse files
authored
fix: use diagnostic name by default (#236)
Corresponds to cpp-linter/cpp-linter#173. This should never happen in the wild, but just in case there's an underlying problem with parsing clang-tidy output... This fixes a problem about hyperlinking a clang-tidy diagnostic name to the clang-tidy docs. Specifically when the diagnostic name does not satisfy the following conditions: - starts with "clang-analyzer-" - starts with "clang-diagnostic-" - contains at least 1 hyphen (`-`) This also removes the `.unwrap()` calls (bad practice) and adds a test for code coverage.
1 parent a4575ec commit cbe77a2

File tree

1 file changed

+34
-11
lines changed

1 file changed

+34
-11
lines changed

cpp-linter/src/clang_tools/clang_tidy.rs

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,28 @@ pub struct TidyNotification {
7474

7575
impl TidyNotification {
7676
pub fn diagnostic_link(&self) -> String {
77-
if self.diagnostic.starts_with("clang-diagnostic") {
77+
if self.diagnostic.starts_with("clang-diagnostic-") {
78+
// clang-diagnostic-* diagnostics are compiler diagnostics and don't have
79+
// dedicated clang-tidy documentation pages, so return the name as-is.
7880
return self.diagnostic.clone();
7981
}
80-
let (category, name) = if self.diagnostic.starts_with("clang-analyzer-") {
81-
(
82-
"clang-analyzer",
83-
self.diagnostic.strip_prefix("clang-analyzer-").unwrap(),
82+
if let Some((category, name)) = if self.diagnostic.starts_with("clang-analyzer-") {
83+
self.diagnostic
84+
.strip_prefix("clang-analyzer-")
85+
.map(|n| ("clang-analyzer", n))
86+
} else {
87+
self.diagnostic.split_once('-')
88+
} {
89+
// In production, both category and name should be non-empty strings.
90+
// Clang does not actually have a diagnostic name whose category or name is empty.
91+
debug_assert!(!category.is_empty() && !name.is_empty());
92+
format!(
93+
"[{}](https://clang.llvm.org/extra/clang-tidy/checks/{category}/{name}.html)",
94+
self.diagnostic
8495
)
8596
} else {
86-
self.diagnostic.split_once('-').unwrap()
87-
};
88-
format!(
89-
"[{}](https://clang.llvm.org/extra/clang-tidy/checks/{category}/{name}.html)",
90-
self.diagnostic
91-
)
97+
self.diagnostic.clone()
98+
}
9299
}
93100
}
94101

@@ -392,6 +399,22 @@ mod test {
392399
assert_eq!(note.diagnostic_link(), expected);
393400
}
394401

402+
#[test]
403+
fn invalid_diagnostic_link() {
404+
let expected = "no_diagnostic_name".to_string();
405+
let note = TidyNotification {
406+
filename: String::from("some_src.cpp"),
407+
line: 1504,
408+
cols: 9,
409+
rationale: String::from("some rationale"),
410+
severity: String::from("warning"),
411+
diagnostic: expected.clone(),
412+
suggestion: vec![],
413+
fixed_lines: vec![],
414+
};
415+
assert_eq!(note.diagnostic_link(), expected);
416+
}
417+
395418
// ***************** test for regex parsing of clang-tidy stdout
396419

397420
#[test]

0 commit comments

Comments
 (0)