Skip to content

Preview/Clang-Tidy #89

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.

Sign up for GitHub

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

Open
andrescastellanos-coderabbit wants to merge 2 commits into
base: main
Choose a base branch
from
+75 −0
Open

Preview/Clang-Tidy #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b69aee
Preview/Clang-Tidy: add C++ sample with clang-tidy config, compile DB…
andrescastellanos-coderabbit Apr 24, 2026
aca8de9
Preview/Clang-Tidy: rewrite to match coderabbit support recipe
andrescastellanos-coderabbit Apr 24, 2026
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
16 changes: 16 additions & 0 deletions .clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Checks: >-
-*,
clang-analyzer-*,
bugprone-*,
modernize-use-nullptr,
modernize-use-override,
cppcoreguidelines-owning-memory,
cppcoreguidelines-pro-type-member-init
WarningsAsErrors: >-
clang-analyzer-*,
bugprone-*,
modernize-use-nullptr,
modernize-use-override,
cppcoreguidelines-owning-memory,
cppcoreguidelines-pro-type-member-init
FormatStyle: none
4 changes: 4 additions & 0 deletions .coderabbit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
reviews:
tools:
clang:
enabled: true
7 changes: 7 additions & 0 deletions compile_commands.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"directory": "/home/jailuser/git",
"file": "test.cpp",
"command": "clang++ -std=c++17 -I. -c test.cpp"
}
]
48 changes: 48 additions & 0 deletions test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <cstdio>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

cc_file="$(fd -i '^compile_commands\.json$' | head -n1)"
[ -n "${cc_file}" ] || { echo "compile_commands.json not found"; exit 1; }

echo "Using: ${cc_file}"
jq -r '
  .[] 
  | select(.file | test("test.cpp$")) 
  | "directory=\(.directory)\ncommand=\(.command // (.arguments|join(" ")))\n"
' "${cc_file}"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 179

🏁 Script executed:

# Check if test.cpp exists and view its contents
if [ -f test.cpp ]; then
  echo "=== test.cpp exists ==="
  head -20 test.cpp
else
  echo "test.cpp does not exist"
fi

Repository: coderabbitai/coderabbit-pr-review

Length of output: 413

🏁 Script executed:

# Try to compile with the exact command from compile_commands.json
cd /home/jailuser/git
clang++ -std=c++17 -I. -c test.cpp 2>&1 | head -50
echo "Exit code: $?"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 197

🏁 Script executed:

# Check if cstdio is available in system paths
echo "Checking for cstdio in system paths:"
find /usr/include -name "cstdio" 2>/dev/null | head -5
find /usr -name "cstdio" -type f 2>/dev/null | head -5

Repository: coderabbitai/coderabbit-pr-review

Length of output: 114

Compilation fails before analysis: missing standard include paths

The compilation command in compile_commands.json for test.cpp omits system include paths. Line 1's #include <cstdio> fails with 'cstdio' file not found, preventing clang-tidy from analyzing this file.

Fix compile_commands.json to include proper system include paths (e.g., via -isystem or by using a compiler driver that provides them automatically).

🧰 Tools
🪛 Clang (14.0.6)

[error] 1-1: 'cstdio' file not found

(clang-diagnostic-error)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` at line 1, The compile_commands.json entry for test.cpp is missing
system include paths so `#include` <cstdio> cannot be found; update the compile
command for test.cpp in compile_commands.json to invoke a proper compiler driver
or add the standard system include directories (e.g., add -isystem
<path-to-stdlib-headers> or use the same gcc/clang binary used for builds) so
the command provides standard headers; target the compile_commands.json entry
for "test.cpp" and ensure the "command" or "arguments" field includes the
correct system include flags or uses the compiler executable that supplies them.

#include <cstdlib>
#include <cstring>
#include <iostream>

class Shape {
public:
Shape() {}
virtual void draw() {}
~Shape() {}
private:
int size;
int color;
};
Comment on lines +6 to +14
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non-virtual destructor with virtual methods causes undefined behavior.

Shape has a virtual method draw() but a non-virtual destructor. When main() deletes a Circle through a Shape* pointer (line 41 allocates, but no delete is shown—itself a leak), destroying via the base pointer would invoke undefined behavior.

Additionally, the constructor on line 8 does not initialize the size and color members, which clang-tidy flags via cppcoreguidelines-pro-type-member-init.

Proposed fix 
 class Shape {
 public:
-    Shape() {}
+    Shape() : size(0), color(0) {}
     virtual void draw() {}
-    ~Shape() {}
+    virtual ~Shape() = default;
 private:
     int size;
     int color;
 };
🧰 Tools
🪛 Clang (14.0.6)

[error] 8-8: constructor does not initialize these fields: size, color

(cppcoreguidelines-pro-type-member-init,-warnings-as-errors)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 6 - 14, The Shape class declares a virtual method
draw() but has a non-virtual destructor and uninitialized members; change
Shape's destructor to virtual (e.g., virtual ~Shape() = default;) so deleting
derived objects via a Shape* is well-defined, and initialize size and color in
Shape's constructor (use an initializer list or in-class member initializers) to
satisfy cppcoreguidelines-pro-type-member-init; update any places that new
Circle(...) and delete via Shape* in main() to rely on the virtual destructor.


class Circle : public Shape {
public:
Circle() {}
void draw() { std::cout << "circle" << std::endl; }
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd / && find . -name "test.cpp" -type f 2>/dev/null | head -5

Repository: coderabbitai/coderabbit-pr-review

Length of output: 106

🏁 Script executed:

cat -n ./home/jailuser/git/test.cpp

Repository: coderabbitai/coderabbit-pr-review

Length of output: 138

🏁 Script executed:

git ls-files | grep test.cpp

Repository: coderabbitai/coderabbit-pr-review

Length of output: 86

🏁 Script executed:

cat -n test.cpp

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1137

Add override keyword to explicitly mark the overridden method

Line 19 overrides the virtual void draw() method from the base Shape class. Use override to enforce signature correctness and satisfy modern C++ best practices.

Proposed fix 
-    void draw() { std::cout << "circle" << std::endl; }
+    void draw() override { std::cout << "circle" << std::endl; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void draw() { std::cout << "circle" << std::endl; }
void draw() override { std::cout << "circle" << std::endl; }
🧰 Tools
🪛 Clang (14.0.6)

[error] 19-19: annotate this function with 'override' or (rarely) 'final'

(modernize-use-override,-warnings-as-errors)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` at line 19, The draw() method currently overrides Shape::draw() but
is missing the override specifier; update the method declaration for draw (the
function named draw in the derived class) to append the override keyword so the
compiler enforces signature matching (i.e., change the declaration of void
draw() to explicitly be declared with override).

};

void nullDeref(int* p) {
if (p == NULL) {
*p = 5;
}
Comment on lines +22 to +25
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find test.cpp in the repository
find . -name "test.cpp" -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 88

🏁 Script executed:

# Read test.cpp to verify lines 22-25
cat -n test.cpp | head -30

Repository: coderabbitai/coderabbit-pr-review

Length of output: 754

Null check is inverted and dereferences null

At line 23, when p == NULL, line 24 dereferences it. This causes an immediate crash.

Proposed fix 
 void nullDeref(int* p) {
-    if (p == NULL) {
+    if (p != nullptr) {
         *p = 5;
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void nullDeref(int* p) {
if (p == NULL) {
*p = 5;
}
void nullDeref(int* p) {
if (p != nullptr) {
*p = 5;
}
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 22 - 25, In function nullDeref, the null-check is
inverted: the code dereferences p when p == NULL which crashes; change the logic
to only dereference p when it is non-null (e.g., if (p != NULL) { *p = 5; } ) or
handle the null case explicitly (return or report error) so that *p is never
executed on a null pointer.

}

int divide(int numerator, int denominator) {
return numerator / denominator;
}
Comment on lines +28 to +30
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat test.cpp

Repository: coderabbitai/coderabbit-pr-review

Length of output: 801

Deterministic division-by-zero path

divide has no denominator guard (Line 29), and main calls it with 0 (Line 36). This is guaranteed UB/crash.

Proposed fix 
+#include <stdexcept>
 int divide(int numerator, int denominator) {
+    if (denominator == 0) {
+        throw std::invalid_argument("denominator must not be zero");
+    }
     return numerator / denominator;
 }
@@
-    int x = divide(100, 0);
+    int x = divide(100, 5);
🧰 Tools
🪛 Cppcheck (2.20.0)

[error] 29-29: Division by zero.

(zerodiv)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 28 - 30, The function divide has no guard against a
zero denominator (divide) and main calls it with 0, causing deterministic UB;
add a check inside divide to detect denominator == 0 and handle it (e.g., throw
std::invalid_argument or return an error code/optional) and then update the
caller in main to handle that error path (catch the exception or check the
return/optional) so the division-by-zero is never executed. Ensure you modify
the divide signature/behavior consistently (throwing exception or using
std::optional/int error sentinel) and adjust main's call site to handle the
chosen error strategy.


int main() {
int uninitialized;
int result = uninitialized + 7;
Comment on lines +33 to +34
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reading uninitialized local variable is undefined behavior.

uninitialized is declared but never assigned a value before being read on line 34. This is flagged by static analysis (uninitvar).

Proposed fix 
-    int uninitialized;
-    int result = uninitialized + 7;
+    int initialized = 0;
+    int result = initialized + 7;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
int uninitialized;
int result = uninitialized + 7;
int initialized = 0;
int result = initialized + 7;
🧰 Tools
🪛 Cppcheck (2.20.0)

[error] 34-34: Uninitialized variable

(uninitvar)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 33 - 34, The variable 'uninitialized' is read before
being assigned, causing undefined behavior; initialize it prior to use or assign
it a value before computing 'result' (e.g., set 'uninitialized' to a sensible
default or compute/obtain its value earlier) so that 'int result = uninitialized
+ 7;' uses a defined value; update the declaration or the code path that sets
'uninitialized' accordingly.


int x = divide(100, 0);

int* p = NULL;
nullDeref(p);

Shape* s = new Circle();
s->draw();
Comment on lines +41 to +42
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Memory leak: raw new without corresponding delete.

new Circle() allocates heap memory that is never freed. Prefer smart pointers (std::unique_ptr) to ensure automatic cleanup, which also aligns with the enabled cppcoreguidelines-owning-memory check.

Proposed fix 
+#include <memory>
 // ...
-    Shape* s = new Circle();
-    s->draw();
+    auto s = std::make_unique<Circle>();
+    s->draw();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Shape* s = new Circle();
s->draw();
`#include` <memory>
auto s = std::make_unique<Circle>();
s->draw();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 41 - 42, The code leaks heap memory by using raw new
for Circle assigned to Shape* s without delete; replace the owning raw pointer
with a std::unique_ptr<Shape> (or std::unique_ptr<Circle> if specific) and
construct it with std::make_unique so the destructor runs automatically, then
call draw via the smart pointer (e.g., s->draw()); update any declarations of s
and includes (add <memory>) accordingly to remove the manual delete requirement.


const char* fmt = "%s";
printf(fmt);
Comment on lines +44 to +45
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "test.cpp" -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 88

🏁 Script executed:

cat -n ./test.cpp

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1137

printf format/argument mismatch

Line 45 passes a %s format string with zero arguments. This is undefined behavior.

Proposed fix 
-    printf(fmt);
+    std::printf("%s", fmt);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const char* fmt = "%s";
printf(fmt);
const char* fmt = "%s";
std::printf("%s", fmt);
🧰 Tools
🪛 Cppcheck (2.20.0)

[error] 45-45: printf format string requires 1 parameter but only 0 are given.

(wrongPrintfScanfArgNum)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test.cpp` around lines 44 - 45, The printf call uses format string variable
fmt containing "%s" but supplies no argument, causing undefined behavior; fix by
providing a matching argument (e.g., printf(fmt, some_c_string) or printf("%s",
fmt) if you intended to print the fmt value) or replace with puts(fmt) if you
just want to output the string. Update the call near the fmt variable and
printf(...) to pass the correct C string argument or use a non-format output
function.


return result + x;
}