Skip to content
Merged
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `ObjectPassedAsInterface` analysis rule, which detects object references that are passed directly as
an interface to a routine.
- `TooManyDefaultParameters` analysis rule, which flags routines with an excessive number of default parameters.

### Changed

- `EmptyBlock` now ignores all empty blocks containing an explanatory comment.
- `TooManyParameters` now flags routine declarations instead of implementations, improving support for methods
that are virtual, abstract, or on an interface.

## [1.17.2] - 2025-07-03

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public final class CheckList {
TabulationCharacterCheck.class,
TooLargeRoutineCheck.class,
TooLongLineCheck.class,
TooManyDefaultParametersCheck.class,
TooManyNestedRoutinesCheck.class,
TooManyParametersCheck.class,
TooManyVariablesCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Sonar Delphi Plugin
* Copyright (C) 2025 Integrated Application Development
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package au.com.integradev.delphi.checks;

import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.plugins.communitydelphi.api.ast.FormalParameterNode.FormalParameterData;
import org.sonar.plugins.communitydelphi.api.ast.RoutineDeclarationNode;
import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
import org.sonar.plugins.communitydelphi.api.ast.RoutineNameNode;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
import org.sonar.plugins.communitydelphi.api.type.Parameter;

@Rule(key = "TooManyDefaultParameters")
public class TooManyDefaultParametersCheck extends DelphiCheck {
private static final int DEFAULT_MAXIMUM = 2;

@RuleProperty(
key = "max",
description = "Maximum authorized number of default parameters",
defaultValue = "" + DEFAULT_MAXIMUM)
public int max = DEFAULT_MAXIMUM;

@Override
public DelphiCheckContext visit(RoutineDeclarationNode routine, DelphiCheckContext context) {
var count =
routine.getParameters().stream().filter(FormalParameterData::hasDefaultValue).count();
checkRoutine(routine.getRoutineNameNode(), (int) count, max, context);
return super.visit(routine, context);
}

@Override
public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) {
var declaration = routine.getRoutineNameDeclaration();
if (declaration == null || !declaration.isImplementationDeclaration()) {
// Don't duplicate issues between the declaration and implementation
return super.visit(routine, context);
}

// A routine implementation's default parameters MUST match its declaration's, or not have any
// default parameters at all. This means that we need to check the declaration to get the
// authoritative number of default parameters.
var count = declaration.getParameters().stream().filter(Parameter::hasDefaultValue).count();
checkRoutine(routine.getRoutineNameNode(), (int) count, max, context);
return super.visit(routine, context);
}

private void checkRoutine(RoutineNameNode node, int count, int max, DelphiCheckContext context) {
if (count > max) {
reportIssue(
context,
node,
String.format(
"Routine has %d default parameters, which is greater than %d authorized.",
count, max));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.plugins.communitydelphi.api.ast.RoutineDeclarationNode;
import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
import org.sonar.plugins.communitydelphi.api.ast.RoutineNameNode;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
Expand All @@ -42,18 +44,45 @@ public class TooManyParametersCheck extends DelphiCheck {
defaultValue = "" + DEFAULT_MAXIMUM)
public int constructorMax = DEFAULT_MAXIMUM;

@Override
public DelphiCheckContext visit(RoutineDeclarationNode routine, DelphiCheckContext context) {
checkRoutine(
routine.getRoutineNameNode(),
routine.getParameters().size(),
routine.isConstructor(),
context);
return super.visit(routine, context);
}

@Override
public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) {
int count = routine.getParameters().size();
int limit = routine.isConstructor() ? constructorMax : max;
if (count > limit) {
var declaration = routine.getRoutineNameDeclaration();
if (declaration == null || !declaration.isImplementationDeclaration()) {
// Don't duplicate issues between the declaration and implementation
return super.visit(routine, context);
}

// A routine implementation's default parameters MUST match its declaration's, or not have any
// default parameters at all. This means that we need to check the declaration to get the
// authoritative number of default parameters.
checkRoutine(
routine.getRoutineNameNode(),
declaration.getParameters().size(),
routine.isConstructor(),
context);
return super.visit(routine, context);
}

private void checkRoutine(
RoutineNameNode node, int count, boolean isConstructor, DelphiCheckContext context) {
var thisMax = isConstructor ? constructorMax : max;
if (count > thisMax) {
reportIssue(
context,
routine.getRoutineNameNode(),
node,
String.format(
"%s has %d parameters, which is greater than %d authorized.",
routine.isConstructor() ? "Constructor" : "Routine", count, limit));
isConstructor ? "Constructor" : "Routine", count, thisMax));
}
return super.visit(routine, context);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
"TabulationCharacter",
"TooLargeRoutine",
"TooLongLine",
"TooManyDefaultParameters",
"TooManyNestedRoutines",
"TooManyParameters",
"TrailingCommaArgumentList",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<h2>Why is this an issue?</h2>
<p>
Though default parameters can be useful in cases when there is a sensible default value, they also damage API
interfaces when used to make a method's behaviour overly configurable. A method with too many default parameters
is confusing, unpredictable at a glance, unintuitive to use, and likely overloaded in functionality.
</p>
<p>
In most cases, using method overloads, multiple methods, or configuration objects is a more
appropriate solution.
</p>
<h2>How to fix it</h2>
<p>
Refactor this routine by re-evaluating the need for the default values, and either:
</p>
<ul>
<li>Removing the unnecessary default values,</li>
<li>Adding multiple overloads with different groups of required parameters, or</li>
<li>Grouping related parameters into a configuration object</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"title": "Routines should not have too many default parameters",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant/Issue",
"constantCost": "5min"
},
"code": {
"attribute": "FOCUSED",
"impacts": {
"MAINTAINABILITY": "MEDIUM"
}
},
"tags": ["brain-overload"],
"defaultSeverity": "Major",
"scope": "MAIN",
"quickfix": "unknown"
}
Loading