Skip to content

Add PrimaryConstructorLastRule to enforce constructor ordering #1332

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 6 commits into
base: master
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
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<img alt="logo" src="https://www.qulice.com/logo.svg" width="200px" height="55px"/>
# Qulice

![logo](https://www.qulice.com/logo.svg)
[![EO principles respected here](https://www.elegantobjects.org/badge.svg)](https://www.elegantobjects.org)
[![DevOps By Rultor.com](https://www.rultor.com/b/yegor256/qulice)](https://www.rultor.com/p/yegor256/qulice)
[![We recommend IntelliJ IDEA](https://www.elegantobjects.org/intellij-idea.svg)](https://www.jetbrains.com/idea/)
Expand Down Expand Up @@ -72,7 +73,7 @@ they don't violate our quality standards. To avoid frustration, before
sending us your pull request please run full Maven build:

```bash
$ mvn clean install -Pqulice
mvn clean install -Pqulice
```

Keep in mind that JDK 11+ and Maven 3.8+ are the lowest versions you may use.
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/**
* SPDX-FileCopyrightText: Copyright (c) 2011-2025 Yegor Bugayenko
* SPDX-License-Identifier: MIT
* Check that primary constructor is placed at the end of constructors list.
*
* This rule checks that the constructor with the most parameters (primary constructor)
* is placed after all other constructors in the class.
*/
package com.qulice.pmd.rules;

import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;

import java.util.ArrayList;
import java.util.List;
import java.util.Comparator;

/**
* Rule that checks primary constructor is placed at the end.
* Primary constructor is the one with the most parameters.
*
* @since 0.18
*/
public final class PrimaryConstructorLastRule extends AbstractJavaRule {

/**
* Error message for the rule violation.
*/
private static final String MESSAGE =
"Primary constructor (with most parameters) should be placed at the end of constructors list";

@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
// Only check classes, not interfaces
if (!node.isInterface()) {
checkConstructorOrder(node, data);
}
return super.visit(node, data);
}

/**
* Check the order of constructors in the class.
*
* @param classNode Class declaration node
* @param data Rule context data
*/
private void checkConstructorOrder(ASTClassOrInterfaceDeclaration classNode, Object data) {
List<ASTConstructorDeclaration> constructors =
classNode.findDescendantsOfType(ASTConstructorDeclaration.class);

// Filter only direct children constructors (not nested class constructors)
List<ASTConstructorDeclaration> directConstructors = new ArrayList<>();
for (ASTConstructorDeclaration constructor : constructors) {
if (constructor.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) == classNode) {
directConstructors.add(constructor);
}
}

if (directConstructors.size() <= 1) {
// No need to check order if there's only one or no constructors
return;
}

// Find the primary constructor (with most parameters)
ASTConstructorDeclaration primaryConstructor = findPrimaryConstructor(directConstructors);
if (primaryConstructor == null) {
return;
}

// Check if primary constructor is the last one
ASTConstructorDeclaration lastConstructor = directConstructors.get(directConstructors.size() - 1);
if (primaryConstructor != lastConstructor) {
asCtx(data).addViolation(primaryConstructor, MESSAGE);
}
}

/**
* Find primary constructor (the one with most parameters).
* If there are multiple constructors with the same max parameter count,
* consider the first one as primary.
*
* @param constructors List of constructors
* @return Primary constructor or null if no constructors
*/
private ASTConstructorDeclaration findPrimaryConstructor(
List<ASTConstructorDeclaration> constructors) {
if (constructors.isEmpty()) {
return null;
}

return constructors.stream()
.max(Comparator.comparingInt(this::getParameterCount))
.orElse(null);
}

/**
* Get parameter count for a constructor.
*
* @param constructor Constructor declaration
* @return Number of parameters
*/
private int getParameterCount(ASTConstructorDeclaration constructor) {
ASTFormalParameters params = constructor.getFirstDescendantOfType(ASTFormalParameters.class);
return params != null ? params.size() : 0;
}
}
16 changes: 15 additions & 1 deletion qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@
</property>
</properties>
</rule>
<rule name="PrimaryConstructorLast" message="Primary constructor (with most parameters) should be placed at the end of constructors list" class="com.qulice.pmd.rules.PrimaryConstructorLastRule">
<description>
This rule checks that the constructor with the most parameters (primary constructor)
is placed after all other constructors in the class. This convention improves
code readability by placing the most complete constructor at the end.
</description>
</rule>
<rule name="ProhibitPlainJunitAssertionsRule" message="Avoid using Plain JUnit assertions" class="com.qulice.pmd.rules.ProhibitPlainJunitAssertionsRule">
<description>
Instead of using plain JUnit assertions like org.junit.Assert.assert*
Expand Down Expand Up @@ -102,7 +109,14 @@
<property name="xpath">
<value><![CDATA[
//ClassOrInterfaceBody[count(ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)>1]
[count(ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[BlockStatement])>1]
[count(
ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[
BlockStatement[not(
Statement/StatementExpression/PrimaryExpression/PrimaryPrefix[@ThisModifier="true"]
or Statement/StatementExpression/PrimaryExpression/PrimaryPrefix[@SuperModifier="true"]
)]
]
)>1]
]]></value>
</property>
</properties>
Expand Down Expand Up @@ -212,7 +226,7 @@
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<!--Solve priority confict-->

Check warning on line 229 in qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml

View workflow job for this annotation

GitHub Actions / typos

"confict" should be "conflict".
<property name="xpath">
<value><![CDATA[
//TypeDeclaration
Expand Down
Loading