PR review suggestions

.github/dependabot.yml

@@ -16,15 +16,15 @@ updates:
   - package-ecosystem: pip
     directory: cpp-linter-py/
     schedule:
-      interval: "daily"
+      interval: "weekly"
     groups:
       pip:
         patterns:
           - "*"
   - package-ecosystem: cargo
     directory: /
     schedule:
-      interval: "daily"
+      interval: "weekly"
     ignore:
       - dependency-name: cpp-linter-lib
     groups:

.github/workflows/run-dev-tests.yml

@@ -22,30 +22,11 @@ env:
   RUST_BACKTRACE: 1
 
 jobs:
-  cache-deps:
-    runs-on: ${{ matrix.os }}
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [windows-latest, ubuntu-latest]
-    steps:
-      - run: rustup update --no-self-update
-      - uses: actions/checkout@v4
-      - name: Cache .cargo locked resources
-        uses: actions/cache@v4
-        with:
-          path: ~/.cargo
-          key: ${{ runner.os }}-test-cargo-${{ hashFiles('Cargo.lock') }}
-      - run: cargo fetch
-
   test:
-    needs: [cache-deps]
-    name: test ${{ matrix.os }} w/ clang v${{ matrix.version }}
     strategy:
       fail-fast: false
       matrix:
         os: ['windows-latest', ubuntu-latest]
-        version: ['17', '16', '15', '14', '13', '12', '11', '10', '9', '8', '7']
 
     runs-on: ${{ matrix.os }}
     steps:
@@ -87,60 +68,161 @@ jobs:
         if: runner.os == 'Windows'
         run: choco install ninja
 
-      - name: Install Linux clang dependencies
-        if: runner.os == 'Linux'
-        shell: bash
-        run: |
-          sudo apt-get update
-          # First try installing from default Ubuntu repositories before trying LLVM script
-          if ! sudo apt-get install -y clang-format-${{ matrix.version }} clang-tidy-${{ matrix.version }}; then
-            # This LLVM script will add the relevant LLVM PPA: https://apt.llvm.org/
-            wget https://apt.llvm.org/llvm.sh -O ${{ runner.temp }}/llvm_install.sh
-            chmod +x ${{ runner.temp }}/llvm_install.sh
-            if sudo ${{ runner.temp }}/llvm_install.sh ${{ matrix.version }}; then
-              sudo apt-get install -y clang-format-${{ matrix.version }} clang-tidy-${{ matrix.version }}
-            fi
-          fi
-
-      - name: Install clang-tools
-        run: |
-          python -m pip install clang-tools
-          clang-tools --install ${{ matrix.version }}
-
-      - name: Cache .cargo locked resources
-        uses: actions/cache/restore@v4
-        with:
-          path: ~/.cargo
-          key: ${{ runner.os }}-test-cargo-${{ hashFiles('Cargo.lock') }}
-
-      - name: Collect Coverage
+      - name: Fetch .cargo locked resources
+        run: cargo fetch
+
+      - if: runner.os == 'Linux'
+        run: sudo apt-get update
+
+      - name: Install clang v7
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '7'
+
+      - name: Collect Coverage for clang v7
         working-directory: cpp-linter-lib
         env:
-          CLANG_VERSION: ${{ matrix.version }}
+          CLANG_VERSION: '7'
         run: just test
 
-      - name: Generate Coverage HTML report
+      - name: Install clang v8
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '8'
+
+      - name: Collect Coverage for clang v8
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '8'
+        run: just test
+
+      - name: Install clang v9
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '9'
+
+      - name: Collect Coverage for clang v9
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '9'
+        run: just test
+
+      - name: Install clang v10
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '10'
+
+      - name: Collect Coverage for clang v10
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '10'
+        run: just test
+
+      - name: Install clang 11
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '11'
+
+      - name: Collect Coverage for clang v11
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '11'
+        run: just test
+
+      - name: Install clang 12
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '12'
+
+      - name: Collect Coverage for clang v12
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '12'
+        run: just test
+
+      - name: Install clang 13
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '13'
+
+      - name: Collect Coverage for clang v13
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '13'
+        run: just test
+
+      - name: Install clang 14
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '14'
+
+      - name: Collect Coverage for clang v14
         working-directory: cpp-linter-lib
         env:
-          CLANG_VERSION: ${{ matrix.version }}
+          CLANG_VERSION: '14'
+        run: just test
+
+      - name: Install clang 15
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '15'
+
+      - name: Collect Coverage for clang v15
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '15'
+        run: just test
+
+      - name: Install clang 16
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '16'
+
+      - name: Collect Coverage for clang v16
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '16'
+        run: just test
+
+      - name: Install clang 17
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '17'
+
+      - name: Collect Coverage for clang v17
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '17'
+        run: just test
+
+      - name: Install clang 18
+        uses: cpp-linter/cpp_linter_rs/install-clang-action@pr-reviews
+        with:
+          version: '18'
+
+      - name: Collect Coverage for clang v18
+        working-directory: cpp-linter-lib
+        env:
+          CLANG_VERSION: '18'
+        run: just test --run-ignored=all
+
+      - name: Generate Coverage HTML report
+        working-directory: cpp-linter-lib
         run: just pretty-cov
 
       - name: Upload coverage data
         uses: actions/upload-artifact@v4
         with:
-          name: HTML_report-${{ runner.os }}-clang_v${{ matrix.version }}
+          name: HTML_report-${{ runner.os }}
           path: target/llvm-cov-pretty
 
       - name: Generate Coverage lcov report
-        if: matrix.version == '16' && runner.os == 'Linux'
+        if: runner.os == 'Linux'
         working-directory: cpp-linter-lib
-        env:
-          CLANG_VERSION: ${{ matrix.version }}
-        run: |
-          cargo llvm-cov report --lcov --output-path lcov.info
+        run: just lcov
 
       - uses: codecov/codecov-action@v4
-        if: matrix.version == '16' && runner.os == 'Linux'
+        if: runner.os == 'Linux'
         with:
           token: ${{secrets.CODECOV_TOKEN}}
           files: cpp-linter-lib/lcov.info

codecov.yml

@@ -0,0 +1,11 @@
+coverage:
+  status:
+    patch:
+      default:
+        informational: true
+    project:
+      default:
+        target: auto
+        # adjust accordingly based on how flaky your tests are
+        # this allows a 2% drop from the previous base commit coverage
+        threshold: 2%

cpp-linter-lib/Cargo.toml

@@ -18,7 +18,7 @@ log = "0.4.22"
 openssl = { version = "0.10", features = ["vendored"], optional = true }
 openssl-probe = { version = "0.1", optional = true }
 regex = "1.10.6"
-reqwest = { version = "0.12.7", features = ["json"]}
+reqwest = "0.12.7"
 semver = "1.0.23"
 serde = { version = "1.0.209", features = ["derive"] }
 serde-xml-rs = "0.6.0"

cpp-linter-lib/src/clang_tools/clang_format.rs

@@ -3,16 +3,18 @@
 
 use std::{
     process::Command,
-    sync::{Arc, Mutex},
+    sync::{Arc, Mutex, MutexGuard},
 };
 
+use log::Level;
 // non-std crates
 use serde::Deserialize;
 use serde_xml_rs::de::Deserializer;
 
 // project-specific crates/modules
+use super::MakeSuggestions;
 use crate::{
-    cli::LinesChangedOnly,
+    cli::ClangParams,
     common_fs::{get_line_cols_from_offset, FileObj},
 };
 
@@ -23,6 +25,18 @@ pub struct FormatAdvice {
     /// A list of [`Replacement`]s that clang-tidy wants to make.
     #[serde(rename = "$value")]
     pub replacements: Vec<Replacement>,
+
+    pub patched: Option<Vec<u8>>,
+}
+
+impl MakeSuggestions for FormatAdvice {
+    fn get_suggestion_help(&self, _start_line: u32, _end_line: u32) -> String {
+        String::from("### clang-format suggestions\n")
+    }
+
+    fn get_tool_name(&self) -> String {
+        "clang-format".to_string()
+    }
 }
 
 /// A single replacement that clang-format wants to make.
@@ -79,24 +93,52 @@ pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
 
 /// Run clang-tidy for a specific `file`, then parse and return it's XML output.
 pub fn run_clang_format(
-    cmd: &mut Command,
-    file: &mut Arc<Mutex<FileObj>>,
-    style: &str,
-    lines_changed_only: &LinesChangedOnly,
+    file: &mut MutexGuard<FileObj>,
+    clang_params: &ClangParams,
 ) -> Vec<(log::Level, String)> {
+    let mut cmd = Command::new(clang_params.clang_format_command.as_ref().unwrap());
     let mut logs = vec![];
-    let mut file = file.lock().unwrap();
-    cmd.args(["--style", style, "--output-replacements-xml"]);
-    let ranges = file.get_ranges(lines_changed_only);
+    cmd.args(["--style", &clang_params.style]);
+    let ranges = file.get_ranges(&clang_params.lines_changed_only);
     for range in &ranges {
         cmd.arg(format!("--lines={}:{}", range.start(), range.end()));
     }
     cmd.arg(file.name.to_string_lossy().as_ref());
+    let mut patched = None;
+    if clang_params.format_review {
+        logs.push((
+            Level::Info,
+            format!(
+                "Getting format fixes with \"{} {}\"",
+                clang_params
+                    .clang_format_command
+                    .as_ref()
+                    .unwrap()
+                    .to_str()
+                    .unwrap(),
+                cmd.get_args()
+                    .map(|a| a.to_str().unwrap())
+                    .collect::<Vec<&str>>()
+                    .join(" ")
+            ),
+        ));
+        patched = Some(
+            cmd.output()
+                .expect("Failed to get fixes from clang-format")
+                .stdout,
+        );
+    }
+    cmd.arg("--output-replacements-xml");
     logs.push((
         log::Level::Info,
         format!(
             "Running \"{} {}\"",
-            cmd.get_program().to_string_lossy(),
+            clang_params
+                .clang_format_command
+                .as_ref()
+                .unwrap()
+                .to_str()
+                .unwrap(),
             cmd.get_args()
                 .map(|x| x.to_str().unwrap())
                 .collect::<Vec<&str>>()
@@ -129,7 +171,9 @@ pub fn run_clang_format(
     let mut format_advice = FormatAdvice::deserialize(&mut Deserializer::new(event_reader))
         .unwrap_or(FormatAdvice {
             replacements: vec![],
+            patched: None,
         });
+    format_advice.patched = patched;
     if !format_advice.replacements.is_empty() {
         let mut filtered_replacements = Vec::new();
         for replacement in &mut format_advice.replacements {
@@ -201,6 +245,7 @@ mod tests {
                     cols: None,
                 },
             ],
+            patched: None,
         };
         let config = serde_xml_rs::ParserConfig::new()
             .trim_whitespace(false)