Skip to main content

cpp_linter/clang_tools/
clang_tidy.rs

1//! This module holds functionality specific to running clang-tidy and parsing it's
2//! output.
3
4use std::{
5    env::{consts::OS, current_dir},
6    fs,
7    path::PathBuf,
8    process::Command,
9    sync::{Arc, Mutex, MutexGuard},
10};
11
12// non-std crates
13use anyhow::{Context, Result, anyhow};
14use clang_installer::utils::normalize_path;
15use regex::Regex;
16use serde::Deserialize;
17
18// project-specific modules/crates
19use super::MakeSuggestions;
20use crate::{cli::ClangParams, common_fs::FileObj};
21
22/// Used to deserialize a json compilation database's translation unit.
23///
24/// The only purpose this serves is to normalize relative paths for build systems that
25/// use/need relative paths (ie ninja).
26#[derive(Deserialize, Debug, Clone)]
27pub struct CompilationUnit {
28    /// The directory of the build environment
29    directory: String,
30
31    /// The file path of the translation unit.
32    ///
33    /// Sometimes, this is relative to the build [`CompilationUnit::directory`].
34    ///
35    /// This is typically the path that clang-tidy uses in its stdout (for a dry run).
36    /// So, having this information helps with matching clang-tidy's stdout with the
37    /// repository files.
38    file: String,
39}
40
41/// A structure that represents a single notification parsed from clang-tidy's stdout.
42#[derive(Debug, Clone)]
43pub struct TidyNotification {
44    /// The file's path and name (supposedly relative to the repository root folder).
45    pub filename: String,
46
47    /// The line number from which the notification originated.
48    pub line: u32,
49
50    /// The column offset on the line from which the notification originated.
51    pub cols: u32,
52
53    /// The severity (ie error/warning/note) of the [`TidyNotification::diagnostic`]
54    /// that caused the notification.
55    pub severity: String,
56
57    /// A helpful message explaining why the notification exists.
58    pub rationale: String,
59
60    /// The diagnostic name as used when configuring clang-tidy.
61    pub diagnostic: String,
62
63    /// A code block that points directly to the origin of the notification.
64    ///
65    /// Sometimes, this code block doesn't exist. Sometimes, it contains suggested
66    /// fixes/advice. This information is purely superfluous.
67    pub suggestion: Vec<String>,
68
69    /// The list of line numbers that had fixes applied via `clang-tidy --fix-error`.
70    pub fixed_lines: Vec<u32>,
71}
72
73impl TidyNotification {
74    pub fn diagnostic_link(&self) -> String {
75        if self.diagnostic.starts_with("clang-diagnostic-") {
76            // clang-diagnostic-* diagnostics are compiler diagnostics and don't have
77            // dedicated clang-tidy documentation pages, so return the name as-is.
78            return self.diagnostic.clone();
79        }
80        if let Some((category, name)) = if self.diagnostic.starts_with("clang-analyzer-") {
81            self.diagnostic
82                .strip_prefix("clang-analyzer-")
83                .map(|n| ("clang-analyzer", n))
84        } else {
85            self.diagnostic.split_once('-')
86        } {
87            // In production, both category and name should be non-empty strings.
88            // Clang does not actually have a diagnostic name whose category or name is empty.
89            debug_assert!(!category.is_empty() && !name.is_empty());
90            format!(
91                "[{}](https://clang.llvm.org/extra/clang-tidy/checks/{category}/{name}.html)",
92                self.diagnostic
93            )
94        } else {
95            self.diagnostic.clone()
96        }
97    }
98}
99
100/// A struct to hold notification from clang-tidy about a single file
101#[derive(Debug, Clone)]
102pub struct TidyAdvice {
103    /// A list of notifications parsed from clang-tidy stdout.
104    pub notes: Vec<TidyNotification>,
105    pub patched: Option<Vec<u8>>,
106}
107
108impl MakeSuggestions for TidyAdvice {
109    fn get_suggestion_help(&self, start_line: u32, end_line: u32) -> String {
110        let mut diagnostics = vec![];
111        for note in &self.notes {
112            for fixed_line in &note.fixed_lines {
113                if (start_line..=end_line).contains(fixed_line) {
114                    diagnostics.push(format!(
115                        "- {} [{}]\n",
116                        note.rationale,
117                        note.diagnostic_link()
118                    ));
119                }
120            }
121        }
122        format!(
123            "### clang-tidy {}\n{}",
124            if diagnostics.is_empty() {
125                "suggestion"
126            } else {
127                "diagnostic(s)"
128            },
129            diagnostics.join("")
130        )
131    }
132
133    fn get_tool_name(&self) -> String {
134        "clang-tidy".to_string()
135    }
136}
137
138/// A regex pattern to capture the clang-tidy note header.
139const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[^\]]*\]$";
140
141/// Parses clang-tidy stdout.
142///
143/// Here it helps to have the JSON database deserialized for normalizing paths present
144/// in the notifications.
145fn parse_tidy_output(
146    tidy_stdout: &[u8],
147    database_json: &Option<Vec<CompilationUnit>>,
148) -> Result<TidyAdvice> {
149    let note_header = Regex::new(NOTE_HEADER)
150        .with_context(|| "Failed to compile RegExp pattern for note header")?;
151    let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")
152        .with_context(|| "Failed to compile RegExp pattern for fixed note")?;
153    let mut found_fix = false;
154    let mut notification = None;
155    let mut result = Vec::new();
156    let cur_dir = current_dir().with_context(|| "Failed to access current working directory")?;
157    for line in String::from_utf8(tidy_stdout.to_vec())
158        .with_context(|| "Failed to convert clang-tidy stdout to UTF-8 string")?
159        .lines()
160    {
161        if let Some(captured) = note_header.captures(line) {
162            // First check that the diagnostic name is a actual diagnostic name.
163            // Sometimes clang-tidy uses square brackets to enclose additional context
164            // about the diagnostic rationale. For example: '[with auto = typename ...]'
165            // We need to ignore such cases as they do not start a diagnostic report.
166            if captured
167                .get(6)
168                .is_some_and(|diag| !diag.as_str().contains(' ') && diag.as_str().contains('-'))
169            {
170                // starting a new TidyNotification; store the previous one (if any) to results
171                if let Some(note) = notification {
172                    result.push(note);
173                }
174
175                // normalize the filename path and try to make it relative to the repo root
176                let mut filename = PathBuf::from(&captured[1]);
177                // if database was given try to use that first
178                if let Some(db_json) = &database_json {
179                    let mut found_unit = false;
180                    for unit in db_json {
181                        let unit_path =
182                            PathBuf::from_iter([unit.directory.as_str(), unit.file.as_str()]);
183                        if unit_path == filename {
184                            filename =
185                                normalize_path(&PathBuf::from_iter([&unit.directory, &unit.file]));
186                            found_unit = true;
187                            break;
188                        }
189                    }
190                    if !found_unit {
191                        // file was not a named unit in the database;
192                        // try to normalize path as if relative to working directory.
193                        // NOTE: This shouldn't happen with a properly formed JSON database
194                        filename = normalize_path(&PathBuf::from_iter([&cur_dir, &filename]));
195                    }
196                } else {
197                    // still need to normalize the relative path despite missing database info.
198                    // let's assume the file is relative to current working directory.
199                    filename = normalize_path(&PathBuf::from_iter([&cur_dir, &filename]));
200                }
201                debug_assert!(filename.is_absolute());
202                if filename.is_absolute()
203                    && let Ok(file_n) = filename.strip_prefix(&cur_dir)
204                {
205                    // if this filename can't be made into a relative path, then it is
206                    // likely not a member of the project's sources (ie /usr/include/stdio.h)
207                    filename = file_n.to_path_buf();
208                }
209
210                notification = Some(TidyNotification {
211                    filename: filename.to_string_lossy().to_string().replace('\\', "/"),
212                    line: captured[2].parse()?,
213                    cols: captured[3].parse()?,
214                    severity: String::from(&captured[4]),
215                    rationale: String::from(&captured[5]).trim().to_string(),
216                    diagnostic: String::from(&captured[6]),
217                    suggestion: Vec::new(),
218                    fixed_lines: Vec::new(),
219                });
220                // begin capturing subsequent lines as suggestions
221                found_fix = false;
222            }
223        } else if let Some(capture) = fixed_note.captures(line) {
224            let fixed_line = capture[1].parse()?;
225            if let Some(note) = &mut notification
226                && !note.fixed_lines.contains(&fixed_line)
227            {
228                note.fixed_lines.push(fixed_line);
229            }
230            // Suspend capturing subsequent lines as suggestions until
231            // a new notification is constructed. If we found a note about applied fixes,
232            // then the lines of suggestions for that notification have already been parsed.
233            found_fix = true;
234        } else if !found_fix && let Some(note) = &mut notification {
235            // append lines of code that are part of
236            // the previous line's notification
237            note.suggestion.push(line.to_string());
238        }
239    }
240    if let Some(note) = notification {
241        result.push(note);
242    }
243    Ok(TidyAdvice {
244        notes: result,
245        patched: None,
246    })
247}
248
249/// Get a total count of clang-tidy advice from the given list of [FileObj]s.
250pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String> {
251    let mut total = 0;
252    for file in files {
253        let file = file.lock().map_err(|e| e.to_string())?;
254        if let Some(advice) = &file.tidy_advice {
255            for tidy_note in &advice.notes {
256                let file_path = PathBuf::from(&tidy_note.filename);
257                if file_path == file.name {
258                    total += 1;
259                }
260            }
261        }
262    }
263    Ok(total)
264}
265
266/// Run clang-tidy, then parse and return it's output.
267pub fn run_clang_tidy(
268    file: &mut MutexGuard<FileObj>,
269    clang_params: &ClangParams,
270) -> Result<Vec<(log::Level, std::string::String)>> {
271    let cmd_path = clang_params
272        .clang_tidy_command
273        .as_ref()
274        .ok_or(anyhow!("clang-tidy command not located"))?;
275    let mut cmd = Command::new(cmd_path);
276    let mut logs = vec![];
277    if !clang_params.tidy_checks.is_empty() {
278        cmd.args(["-checks", &clang_params.tidy_checks]);
279    }
280    if let Some(db) = &clang_params.database {
281        cmd.args(["-p", &db.to_string_lossy()]);
282    }
283    for arg in &clang_params.extra_args {
284        cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
285    }
286    let file_name = file.name.to_string_lossy().to_string();
287    let ranges = file.get_ranges(&clang_params.lines_changed_only);
288    if !ranges.is_empty() {
289        let filter = format!(
290            "[{{\"name\":{:?},\"lines\":{:?}}}]",
291            &file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
292            ranges
293                .iter()
294                .map(|r| [r.start(), r.end()])
295                .collect::<Vec<_>>()
296        );
297        cmd.args(["--line-filter", filter.as_str()]);
298    }
299    let original_content = if !clang_params.tidy_review {
300        None
301    } else {
302        cmd.arg("--fix-errors");
303        Some(fs::read_to_string(&file.name).with_context(|| {
304            format!(
305                "Failed to cache file's original content before applying clang-tidy changes: {}",
306                file_name.clone()
307            )
308        })?)
309    };
310    if !clang_params.style.is_empty() {
311        cmd.args(["--format-style", clang_params.style.as_str()]);
312    }
313    cmd.arg(file.name.to_string_lossy().as_ref());
314    logs.push((
315        log::Level::Info,
316        format!(
317            "Running \"{} {}\"",
318            cmd.get_program().to_string_lossy(),
319            cmd.get_args()
320                .map(|x| x.to_string_lossy())
321                .collect::<Vec<_>>()
322                .join(" ")
323        ),
324    ));
325    let output = cmd.output().with_context(|| {
326        format!(
327            "Failed to execute clang-tidy on file: {}",
328            file_name.clone()
329        )
330    })?;
331    logs.push((
332        log::Level::Debug,
333        format!(
334            "Output from clang-tidy:\n{}",
335            String::from_utf8_lossy(&output.stdout)
336        ),
337    ));
338    if !output.stderr.is_empty() {
339        logs.push((
340            log::Level::Debug,
341            format!(
342                "clang-tidy made the following summary:\n{}",
343                String::from_utf8_lossy(&output.stderr)
344            ),
345        ));
346    }
347    file.tidy_advice = Some(parse_tidy_output(
348        &output.stdout,
349        &clang_params.database_json,
350    )?);
351    if clang_params.tidy_review
352        && let Some(original_content) = &original_content
353    {
354        if let Some(tidy_advice) = &mut file.tidy_advice {
355            // cache file changes in a buffer and restore the original contents for further analysis
356            tidy_advice.patched =
357                Some(fs::read(&file_name).with_context(|| {
358                    format!("Failed to read changes from clang-tidy: {file_name}")
359                })?);
360        }
361        // original_content is guaranteed to be Some() value at this point
362        fs::write(&file_name, original_content)
363            .with_context(|| format!("Failed to restore file's original content: {file_name}"))?;
364    }
365    Ok(logs)
366}
367
368#[cfg(test)]
369mod test {
370    #![allow(clippy::unwrap_used)]
371
372    use std::{
373        env,
374        path::PathBuf,
375        str::FromStr,
376        sync::{Arc, Mutex},
377    };
378
379    use clang_installer::RequestedVersion;
380    use regex::Regex;
381
382    use crate::{
383        clang_tools::{ClangTool, clang_tidy::parse_tidy_output},
384        cli::{ClangParams, LinesChangedOnly},
385        common_fs::FileObj,
386    };
387
388    use super::{NOTE_HEADER, TidyNotification, run_clang_tidy};
389
390    #[test]
391    fn clang_diagnostic_link() {
392        let note = TidyNotification {
393            filename: String::from("some_src.cpp"),
394            line: 1504,
395            cols: 9,
396            rationale: String::from("file not found"),
397            severity: String::from("error"),
398            diagnostic: String::from("clang-diagnostic-error"),
399            suggestion: vec![],
400            fixed_lines: vec![],
401        };
402        assert_eq!(note.diagnostic_link(), note.diagnostic);
403    }
404
405    #[test]
406    fn clang_analyzer_link() {
407        let note = TidyNotification {
408            filename: String::from("some_src.cpp"),
409            line: 1504,
410            cols: 9,
411            rationale: String::from(
412                "Dereference of null pointer (loaded from variable 'pipe_num')",
413            ),
414            severity: String::from("warning"),
415            diagnostic: String::from("clang-analyzer-core.NullDereference"),
416            suggestion: vec![],
417            fixed_lines: vec![],
418        };
419        let expected = format!(
420            "[{}](https://clang.llvm.org/extra/clang-tidy/checks/{}/{}.html)",
421            note.diagnostic, "clang-analyzer", "core.NullDereference",
422        );
423        assert_eq!(note.diagnostic_link(), expected);
424    }
425
426    #[test]
427    fn invalid_diagnostic_link() {
428        let expected = "no_diagnostic_name".to_string();
429        let note = TidyNotification {
430            filename: String::from("some_src.cpp"),
431            line: 1504,
432            cols: 9,
433            rationale: String::from("some rationale"),
434            severity: String::from("warning"),
435            diagnostic: expected.clone(),
436            suggestion: vec![],
437            fixed_lines: vec![],
438        };
439        assert_eq!(note.diagnostic_link(), expected);
440    }
441
442    // ***************** test for regex parsing of clang-tidy stdout
443
444    #[test]
445    fn test_capture() {
446        let src = "tests/demo/demo.hpp:11:11: \
447        warning: use a trailing return type for this function \
448        [modernize-use-trailing-return-type,-warnings-as-errors]";
449        let pat = Regex::new(NOTE_HEADER).unwrap();
450        let cap = pat.captures(src).unwrap();
451        assert_eq!(
452            cap.get(0).unwrap().as_str(),
453            format!(
454                "{}:{}:{}: {}:{}[{},-warnings-as-errors]",
455                cap.get(1).unwrap().as_str(),
456                cap.get(2).unwrap().as_str(),
457                cap.get(3).unwrap().as_str(),
458                cap.get(4).unwrap().as_str(),
459                cap.get(5).unwrap().as_str(),
460                cap.get(6).unwrap().as_str()
461            )
462            .as_str()
463        )
464    }
465
466    #[test]
467    fn use_extra_args() {
468        let exe_path = ClangTool::ClangTidy
469            .get_exe_path(
470                &RequestedVersion::from_str(
471                    env::var("CLANG_VERSION").unwrap_or("".to_string()).as_str(),
472                )
473                .unwrap(),
474            )
475            .unwrap();
476        let file = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
477        let arc_file = Arc::new(Mutex::new(file));
478        let extra_args = vec!["-std=c++17".to_string(), "-Wall".to_string()];
479        let clang_params = ClangParams {
480            style: "".to_string(),
481            tidy_checks: "".to_string(), // use .clang-tidy config file
482            lines_changed_only: LinesChangedOnly::Off,
483            database: None,
484            extra_args: extra_args.clone(), // <---- the reason for this test
485            database_json: None,
486            format_filter: None,
487            tidy_filter: None,
488            tidy_review: false,
489            format_review: false,
490            clang_tidy_command: Some(exe_path),
491            clang_format_command: None,
492        };
493        let mut file_lock = arc_file.lock().unwrap();
494        let logs = run_clang_tidy(&mut file_lock, &clang_params)
495            .unwrap()
496            .into_iter()
497            .filter_map(|(_lvl, msg)| {
498                if msg.contains("Running ") {
499                    Some(msg)
500                } else {
501                    None
502                }
503            })
504            .collect::<Vec<String>>();
505        let args = &logs
506            .first()
507            .expect("expected a log message about invoked clang-tidy command")
508            .split(' ')
509            .collect::<Vec<&str>>();
510        for arg in &extra_args {
511            let extra_arg = format!("\"{arg}\"");
512            assert!(args.contains(&extra_arg.as_str()));
513        }
514    }
515
516    #[test]
517    fn skip_parse_tidy_diagnostic_rationale() {
518        let tidy_out = r#"
519TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:46:19: error: use of undeclared identifier 'readFreeImageTexture' [clang-diagnostic-error]
520   46 |            return readFreeImageTexture(reader);
521      |                   ^
522TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:659:32: note: in instantiation of function template specialization 'tb::mdl::(anonymous namespace)::loadTexture(const std::string &)::(anonymous class)::operator()<std::shared_ptr<tb::fs::File>>' requested here
523  659 |     using Fn_Result = decltype(f(std::declval<Value&&>()));
524      |                                ^
525TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
526 2949 |   return std::forward<R>(r).and_then(t.and_then);
527      |                             ^
528TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
529   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
530      |                                ^
531TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:661:19: error: static assertion failed due to requirement 'is_result_v<int>': Function must return a result type [clang-diagnostic-error]
532  661 |     static_assert(is_result_v<Fn_Result>, "Function must return a result type");
533      |                   ^~~~~~~~~~~~~~~~~~~~~~
534TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
535 2949 |   return std::forward<R>(r).and_then(t.and_then);
536      |                             ^
537TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
538   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
539      |                                ^
540TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:663:77: error: no type named 'type' in 'kdl::detail::chain_results<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, int>' [clang-diagnostic-error]
541  663 |     using Cm_Result = typename detail::chain_results<My_Result, Fn_Result>::type;
542      |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
543TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:667:48: error: no matching function for call to object of type 'const (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)' [clang-diagnostic-error]
544  667 |         [&](value_type&& v) { return Cm_Result{f(std::move(v))}; },
545      |                                                ^
546TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:667:29: note: while substituting into a lambda expression here
547  667 |         [&](value_type&& v) { return Cm_Result{f(std::move(v))}; },
548      |                             ^
549TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
550 2949 |   return std::forward<R>(r).and_then(t.and_then);
551      |                             ^
552TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
553   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
554      |                                ^
555TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48: note: candidate template ignored: substitution failure [with file:auto = typename std::remove_reference<shared_ptr<File> &>::type]
556   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
557      |                                                ^
558"#;
559        let advice = parse_tidy_output(tidy_out.as_bytes(), &None).unwrap();
560        assert_eq!(advice.notes.len(), 4);
561        for note in advice.notes {
562            assert!(note.diagnostic.contains('-'));
563            assert!(!note.diagnostic.contains(' '));
564        }
565    }
566}