Skip to content

Preview/pmd #49

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.

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
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
reviews:
path_filters: ["**/ruleset.xml","ruleset.xml", "**/bad.java", "bad.java"]
24 changes: 24 additions & 0 deletions bad.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
public class BadExample {

private int unusedField;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unused private field.

The field unusedField is declared but never used in the class.

-    private int unusedField;
📝 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
private int unusedField;
🧰 Tools
🪛 PMD (7.15.0)

[Medium] 3-3: Avoid unused private fields such as 'unusedField'. (UnusedPrivateField)

(Best Practices)

🤖 Prompt for AI Agents
In bad.java at line 3, there is a private field named unusedField that is
declared but never used anywhere in the class. Remove this unused private field
declaration to clean up the code and avoid unnecessary clutter.


public BadExample() {
// unnecessary constructor
}
Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unnecessary constructor.

The default constructor adds no value and can be removed.

-    public BadExample() {
-        // unnecessary constructor
-    }
📝 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
public BadExample() {
// unnecessary constructor
}
🧰 Tools
🪛 PMD (7.15.0)

[Medium] 5-5: Avoid unnecessary constructors - the compiler will generate these for you (UnnecessaryConstructor)

(Code Style)

🤖 Prompt for AI Agents
In bad.java around lines 5 to 7, the constructor is unnecessary as it does not
add any functionality. Remove the entire constructor method to rely on the
default constructor provided by Java.


public void DOStuff() {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Follow Java naming conventions for methods.

Method names should start with a lowercase letter.

-    public void DOStuff() {
+    public void doStuff() {
🧰 Tools
🪛 PMD (7.15.0)

[High] 9-9: The instance method name 'DOStuff' doesn't match '[a-z][a-zA-Z0-9]*' (MethodNamingConventions)

(Code Style)

🤖 Prompt for AI Agents
In bad.java at line 9, the method name DOStuff does not follow Java naming
conventions which require method names to start with a lowercase letter. Rename
the method from DOStuff to doStuff to comply with the standard Java naming
conventions.

try {
int FooBar = 1;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Follow Java naming conventions for local variables.

Local variable names should start with a lowercase letter.

-            int FooBar = 1;
+            int fooBar = 1;
📝 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 FooBar = 1;
int fooBar = 1;
🧰 Tools
🪛 PMD (7.15.0)

[High] 11-11: The local variable name 'FooBar' doesn't match '^[a-z][a-zA-Z0-9]*$' (LocalVariableNamingConventions)

(Code Style)

🤖 Prompt for AI Agents
In bad.java at line 11, the local variable name 'FooBar' starts with an
uppercase letter, which violates Java naming conventions. Rename the variable to
start with a lowercase letter, such as 'fooBar', to follow standard Java style
for local variables.

System.out.println("Start");

for (int i = 0; i < 30; i++) {
System.out.println("Line " + i);
}
} catch (Exception e) {
// silently ignored
} finally {
Comment on lines +17 to +19
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid empty catch blocks.

Empty catch blocks silently swallow exceptions, making debugging difficult. At minimum, log the exception.

         } catch (Exception e) {
-            // silently ignored
+            // Log the exception or handle it appropriately
+            e.printStackTrace(); // Or use proper logging
         } finally {
📝 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
} catch (Exception e) {
// silently ignored
} finally {
} catch (Exception e) {
// Log the exception or handle it appropriately
e.printStackTrace(); // Or use proper logging
} finally {
🧰 Tools
🪛 PMD (7.15.0)

[Medium] 17-19: Avoid empty catch blocks (EmptyCatchBlock)

(Error Prone)

🤖 Prompt for AI Agents
In bad.java around lines 17 to 19, the catch block is empty and silently ignores
exceptions, which hinders debugging. Modify the catch block to log the caught
exception using an appropriate logging mechanism or print the stack trace to
ensure the error is recorded and can be diagnosed later.

return; // forbidden in finally block
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Never return from a finally block.

Returning from a finally block suppresses any exceptions thrown in the try or catch blocks and can lead to unexpected behavior.

         } finally {
-            return; // forbidden in finally block
+            // Cleanup code if needed, but no return statement
         }
📝 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
return; // forbidden in finally block
} finally {
// Cleanup code if needed, but no return statement
}
🧰 Tools
🪛 PMD (7.15.0)

[Medium] 20-20: Avoid returning from a finally block (ReturnFromFinallyBlock)

(Error Prone)

🤖 Prompt for AI Agents
In bad.java at line 20, remove the return statement from the finally block
because returning there suppresses exceptions from try or catch blocks. Instead,
restructure the code to avoid returning inside finally, such as moving the
return to the try or catch blocks or using a variable to hold the return value
and returning after the finally block completes.

}
}
}

35 changes: 35 additions & 0 deletions ruleset.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="CodeRabbit PMD Ruleset"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0
http://pmd.sourceforge.net/ruleset_2_0_0.xsd"
description="PMD rules aligned with CodeRabbit standards for general-purpose Java projects.">

<!-- Keep existing valid rules -->
<rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
<rule ref="category/java/errorprone.xml/EmptyCatchBlock"/>
<rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>
<rule ref="category/java/codestyle.xml/UnnecessaryConstructor"/>

<!-- Replace deprecated VariableNamingConventions with specific naming rules -->
<rule ref="category/java/codestyle.xml/LocalVariableNamingConventions">
<properties>
<property name="localVarPattern" value="^[a-z][a-zA-Z0-9]*$"/>
</properties>
</rule>

<!-- Replace deprecated ExcessiveMethodLength with NcssCount -->
<rule ref="category/java/design.xml/NcssCount">
<properties>
<property name="methodReportLevel" value="25"/>
<property name="classReportLevel" value="500"/>
</properties>
</rule>

<!-- Optional: Add other specific naming convention rules -->
<rule ref="category/java/codestyle.xml/FieldNamingConventions"/>
<rule ref="category/java/codestyle.xml/MethodNamingConventions"/>
<rule ref="category/java/codestyle.xml/ClassNamingConventions"/>

</ruleset>