Skip to content

Commit

Permalink
Add 'DigitSeparator' and 'DigitGrouping' rules
Browse files Browse the repository at this point in the history
  • Loading branch information
zaneduffield committed Oct 19, 2023
1 parent d7b7090 commit 5c23acb
Show file tree
Hide file tree
Showing 11 changed files with 489 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support for identifiers prefixed with 2 ampersands (`&&`).
- `OleVariant` overloads for `VarArrayRedim` and `VarClear` intrinsics.
- `InlineAssembly` analysis rule, which flags inline assembly usage.
- `DigitSeparator` analysis rule, which flags numeric literals that should use digit separators to
improve readability.
- `DigitGrouping` analysis rule, which flags numeric literals that use non-standard digit groupings.

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public final class CheckList {
DateFormatSettingsCheck.class,
DestructorNameCheck.class,
DestructorWithoutInheritedCheck.class,
DigitGroupingCheck.class,
DigitSeparatorCheck.class,
EmptyArgumentListCheck.class,
EmptyBlockCheck.class,
EmptyFieldSectionCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Sonar Delphi Plugin
* Copyright (C) 2023 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 java.util.regex.Pattern;
import org.sonar.check.Rule;
import org.sonar.plugins.communitydelphi.api.ast.DecimalLiteralNode;
import org.sonar.plugins.communitydelphi.api.ast.IntegerLiteralNode;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
import org.sonar.plugins.communitydelphi.api.token.DelphiToken;

@Rule(key = "DigitGrouping")
public class DigitGroupingCheck extends DelphiCheck {

private static final Pattern VALID_DEC_UNDERSCORE_PATTERN = Pattern.compile("\\d{1,3}(_\\d{3})*");
private static final Pattern VALID_DEC_FLOAT_UNDERSCORE_PATTERN =
Pattern.compile(VALID_DEC_UNDERSCORE_PATTERN.pattern() + "\\..*");

private static final Pattern VALID_HEX_UNDERSCORE_PATTERN =
Pattern.compile(
"(?i)\\$("
+ "([A-F0-9]{1,2}(_[A-F0-9]{2})*)"
+ "|"
+ "([A-F0-9]{1,4}(_[A-F0-9]{4})*)"
+ ")");

private static final Pattern VALID_BIN_UNDERSCORE_PATTERN =
Pattern.compile(
"%("
+ "([01]{1,2}(_[01]{2})*)"
+ "|"
+ "([01]{1,3}(_[01]{3})*)"
+ "|"
+ "([01]{1,4}(_[01]{4})*)"
+ ")");

public static final String MESSAGE = "Use standard digit groupings in this numeric literal.";

@Override
public DelphiCheckContext visit(IntegerLiteralNode literal, DelphiCheckContext context) {
if (isCheckRelevant(literal.getToken()) && invalidIntegerLiteral(literal)) {
reportIssue(context, literal, MESSAGE);
}

return super.visit(literal, context);
}

@Override
public DelphiCheckContext visit(DecimalLiteralNode literal, DelphiCheckContext context) {
if (isCheckRelevant(literal.getToken()) && invalidFloat(literal)) {
reportIssue(context, literal, MESSAGE);
}

return super.visit(literal, context);
}

private boolean isCheckRelevant(DelphiToken token) {
return token.getImage().contains("_");
}

private static boolean invalidIntegerLiteral(IntegerLiteralNode integerLiteral) {
switch (integerLiteral.getRadix()) {
case 2:
return invalidBin(integerLiteral);
case 10:
return invalidDecimal(integerLiteral);
case 16:
return invalidHex(integerLiteral);
default:
return false;
}
}

private static boolean invalidDecimal(IntegerLiteralNode literalNode) {
return notMatching(VALID_DEC_UNDERSCORE_PATTERN, literalNode.getToken());
}

private static boolean invalidFloat(DecimalLiteralNode literalNode) {
return notMatching(VALID_DEC_FLOAT_UNDERSCORE_PATTERN, literalNode.getToken());
}

private static boolean invalidHex(IntegerLiteralNode literalNode) {
return notMatching(VALID_HEX_UNDERSCORE_PATTERN, literalNode.getToken());
}

private static boolean invalidBin(IntegerLiteralNode literalNode) {
return notMatching(VALID_BIN_UNDERSCORE_PATTERN, literalNode.getToken());
}

private static boolean notMatching(Pattern pattern, DelphiToken token) {
return !pattern.matcher(token.getImage()).matches();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Sonar Delphi Plugin
* Copyright (C) 2023 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.apache.commons.lang3.StringUtils;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.plugins.communitydelphi.api.ast.DecimalLiteralNode;
import org.sonar.plugins.communitydelphi.api.ast.IntegerLiteralNode;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;

@Rule(key = "DigitSeparator")
public class DigitSeparatorCheck extends DelphiCheck {

public static final String MESSAGE = "Separate this long numeric literal with underscores.";

private static final int DEFAULT_MAX_DIGITS_WITHOUT_UNDERSCORES = 4;

@RuleProperty(
key = "maxDigitsWithoutUnderscores",
description = "Maximum number of digits allowed without underscores.",
defaultValue = DEFAULT_MAX_DIGITS_WITHOUT_UNDERSCORES + "")
public int maxDigitsWithoutUnderscores = DEFAULT_MAX_DIGITS_WITHOUT_UNDERSCORES;

@Override
public DelphiCheckContext visit(IntegerLiteralNode literal, DelphiCheckContext context) {
if (isIntMissingUnderscores(literal)) {
reportIssue(context, literal, MESSAGE);
}

return super.visit(literal, context);
}

private boolean isIntMissingUnderscores(IntegerLiteralNode literal) {
return literal.getImage().length() > maxDigitsWithoutUnderscores
&& !StringUtils.contains(literal.getToken().getImage(), '_');
}

@Override
public DelphiCheckContext visit(DecimalLiteralNode literal, DelphiCheckContext context) {
if (isIntegerPartMissingUnderscores(literal.getToken().getImage())) {
reportIssue(context, literal, MESSAGE);
}

return super.visit(literal, context);
}

private boolean isIntegerPartMissingUnderscores(String image) {
String[] split = StringUtils.split(image, '.');
return split.length >= 2
&& split[0].length() > maxDigitsWithoutUnderscores
&& !StringUtils.contains(split[0], '_');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<h2>Why is this an issue?</h2>
<p>From Delphi 11 onwards, digit separators <code>_</code> can be used to split number literals into
groups.</p>
<p>
To improve readability, this rule verifies that number literals using separators have groups of a
standard, homogeneous size.
</p>
<ul>
<li>Decimal literals: groups of 3 digits</li>
<li>Hexadecimal literals: groups of 2 or 4 digits</li>
<li>Binary literals: groups of 2, 3, or 4 digits</li>
</ul>

<b>Noncompliant code example</b>
<pre>
const
WrongGroupSize = 1_0000; // Noncompliant; using groups of 4 instead of 3
MixedGroupSize = $F_7F_FFFF; // Noncompliant; using mixed group sizes
</pre>

<b>Compliant code example</b>
<pre>
const
DecimalInt = 10_000;
HexadecimalInt2 = $7_FF_FF;
HexadecimalInt4 = $7_FFFF;
BinaryInt2 = %0_10_10;
BinaryInt3 = %01_010;
BinaryInt4 = %0_1010;
LongDecimalFloat = 10_000.0;
FloatWithAnySeps = 10.0_0_00_1;
</pre>

<h2>Exceptions</h2>
<p>
Only the integer component of floating point numbers is checked for compliance; the fractional and
exponent parts are ignored.
</p>

<h2>How to fix it</h2>
<ol>
<li>Pick an appropriate, standard digit group size from the above list.</li>
<li>Starting from the right, group the digits in the integer using <code>_</code> between groups.
</li>
</ol>

<h2>Resources</h2>
<ul>
<li>
<a href="https://docwiki.embarcadero.com/RADStudio/Alexandria/en/What's_New#Binary_Literals_and_Digit_Separator">
Delphi Alexandria Release Notes
</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"title": "Digit separators should be used with standard groupings",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant/Issue",
"constantCost": "1min"
},
"code": {
"attribute": "FORMATTED",
"impacts": {
"MAINTAINABILITY": "LOW"
}
},
"tags": [
"convention"
],
"defaultSeverity": "Minor",
"scope": "ALL",
"quickfix": "unknown"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<h2>Why is this an issue?</h2>
<p>From Delphi 11 onwards, digit separators <code>_</code> can be used to split number literals into
groups.</p>
<p>
To improve readability, this rule verifies that numeric literals above a configurable size
in digits (default: 4) use separators.
</p>

<b>Noncompliant code example</b>
<pre>
const
DecimalInt = 10000; // Noncompliant; more than 4 digits without separators
HexadecimalInt = $7FFFF; // Noncompliant; more than 4 digits without separators
BinaryInt = %01010; // Noncompliant; more than 4 digits without separators
Float = 10000.0; // Noncompliant; more than 4 (integer) digits without separators
</pre>

<b>Compliant code example</b>
<pre>
const
DecimalInt = 10_000;
HexadecimalInt = $7_FFFF;
BinaryInt = %0_10_10;
LongDecimalFloat = 10_000.0;
FloatWithAnySeps = 10.0_0_00_1;
FloatWithoutSeps = 10.00001;
</pre>

<h2>Exceptions</h2>
<p>
Only the integer component of floating point numbers is checked for compliance; the fractional and
exponent parts are ignored.
</p>

<h2>How to fix it</h2>
<ol>
<li>Group the digits in the integer using <code>_</code> between groups.</li>
</ol>

<h2>Resources</h2>
<ul>
<li>
<a href="https://docwiki.embarcadero.com/RADStudio/Alexandria/en/What's_New#Binary_Literals_and_Digit_Separator">
Delphi Alexandria Release Notes
</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"title": "Long number literals should be separated with underscores",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant/Issue",
"constantCost": "1min"
},
"code": {
"attribute": "FORMATTED",
"impacts": {
"MAINTAINABILITY": "LOW"
}
},
"tags": [
"convention"
],
"defaultSeverity": "Minor",
"scope": "ALL",
"quickfix": "unknown"
}
Loading

0 comments on commit 5c23acb

Please sign in to comment.