diff --git a/CHANGELOG.md b/CHANGELOG.md index fcce70895..9b616c14a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support for the `LLVM` symbol, which is defined on LLVM-based toolchains from Delphi 12 onward. - Support for the `IOSSIMULATOR` symbol, which is defined on the `DCCIOSSIMARM64` toolchain. +- `FormDfm` analysis rule, which flags VCL forms/frames that lack a `.dfm` resource. - **API:** `CompilerDirectiveParser` now returns a new `ResourceDirective` type when parsing [resource directives](https://docwiki.embarcadero.com/RADStudio/en/Resource_file_(Delphi)). diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/AbstractFormResourceCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/AbstractFormResourceCheck.java new file mode 100644 index 000000000..7fcea11f0 --- /dev/null +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/AbstractFormResourceCheck.java @@ -0,0 +1,81 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2024 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.Optional; +import org.apache.commons.lang3.StringUtils; +import org.sonar.plugins.communitydelphi.api.ast.DelphiAst; +import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.TypeDeclarationNode; +import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; +import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; +import org.sonar.plugins.communitydelphi.api.directive.CompilerDirective; +import org.sonar.plugins.communitydelphi.api.directive.ResourceDirective; +import org.sonar.plugins.communitydelphi.api.token.DelphiToken; +import org.sonar.plugins.communitydelphi.api.type.Type; + +abstract class AbstractFormResourceCheck extends DelphiCheck { + protected abstract String getFrameworkName(); + + protected abstract String getFormTypeImage(); + + protected abstract String getFrameTypeImage(); + + protected abstract String getResourceFileExtension(); + + @Override + public DelphiCheckContext visit(DelphiAst ast, DelphiCheckContext context) { + if (context.getTokens().stream() + .filter(DelphiToken::isCompilerDirective) + .map(token -> context.getCompilerDirectiveParser().parse(token)) + .flatMap(Optional::stream) + .anyMatch(this::isFormResource)) { + return context; + } + return super.visit(ast, context); + } + + @Override + public DelphiCheckContext visit(TypeDeclarationNode declaration, DelphiCheckContext context) { + DelphiNode location = declaration.getTypeNameNode(); + + Type type = declaration.getType(); + if (!type.isAlias()) { + if (type.isDescendantOf(getFormTypeImage())) { + reportIssue(context, location, getMessage("form")); + } else if (type.isDescendantOf(getFrameTypeImage())) { + reportIssue(context, location, getMessage("frame")); + } + } + + return context; + } + + private boolean isFormResource(CompilerDirective directive) { + return directive instanceof ResourceDirective + && StringUtils.endsWithIgnoreCase( + ((ResourceDirective) directive).getResourceFile(), "." + getResourceFileExtension()); + } + + private String getMessage(String componentKind) { + return String.format( + "Add a '.%s' resource for this %s %s.", + getResourceFileExtension(), getFrameworkName(), componentKind); + } +} diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java index c2ca33586..cf13053d0 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java @@ -77,6 +77,7 @@ public final class CheckList { ForbiddenPropertyCheck.class, ForbiddenRoutineCheck.class, ForbiddenTypeCheck.class, + FormDfmCheck.class, FreeAndNilTObjectCheck.class, GotoStatementCheck.class, GroupedFieldDeclarationCheck.class, diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FormDfmCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FormDfmCheck.java new file mode 100644 index 000000000..28d21c08c --- /dev/null +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/FormDfmCheck.java @@ -0,0 +1,45 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2024 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; + +@Rule(key = "FormDfm") +public class FormDfmCheck extends AbstractFormResourceCheck { + + @Override + protected String getFrameworkName() { + return "VCL"; + } + + @Override + protected String getFormTypeImage() { + return "Vcl.Forms.TForm"; + } + + @Override + protected String getFrameTypeImage() { + return "Vcl.Forms.TFrame"; + } + + @Override + protected String getResourceFileExtension() { + return "dfm"; + } +} diff --git a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.html b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.html new file mode 100644 index 000000000..b75364566 --- /dev/null +++ b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.html @@ -0,0 +1,52 @@ +

Why is this an issue?

+

+ Omitting the corresponding .dfm resource for a VCL form or frame is typically a + programming error. While there are some cases that won't cause problems at runtime, it remains + unintuitive and confusing. +

+

+ Additionally, a unit that doesn't contain a .dfm resource will lack design-time form + editing functionality. +

+

+ This rule flags any TForm or TFrame descendant in a unit that doesn't + include a .dfm resource. +

+

How to fix it

+

Add a .dfm resource to the unit:

+
+unit Foo;
+
+interface
+
+uses
+  Vcl.Forms;
+
+type
+  TFoo = class(TForm)
+    // ...
+  end;
+
+implementation
+
+end.
+
+
+unit Foo;
+
+interface
+
+uses
+  Vcl.Forms;
+
+type
+  TFoo = class(TForm)
+    // ...
+  end;
+
+implementation
+
+{$R *.dfm}
+
+end.
+
\ No newline at end of file diff --git a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.json b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.json new file mode 100644 index 000000000..73d90eb70 --- /dev/null +++ b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/FormDfm.json @@ -0,0 +1,19 @@ +{ + "title": "VCL forms and frames should have a corresponding .dfm form file", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant/Issue", + "constantCost": "5min" + }, + "code": { + "attribute": "COMPLETE", + "impacts": { + "MAINTAINABILITY": "MEDIUM" + } + }, + "tags": ["bad-practice", "confusing"], + "defaultSeverity": "Major", + "scope": "ALL", + "quickfix": "unknown" +} diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FormDfmCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FormDfmCheckTest.java new file mode 100644 index 000000000..941cdca69 --- /dev/null +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/FormDfmCheckTest.java @@ -0,0 +1,174 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2024 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 au.com.integradev.delphi.builders.DelphiTestUnitBuilder; +import au.com.integradev.delphi.checks.verifier.CheckVerifier; +import org.junit.jupiter.api.Test; + +class FormDfmCheckTest { + @Test + void testNormalClassWithoutDfmShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" TFoo = class(TObject)") + .appendDecl(" end;")) + .verifyNoIssues(); + } + + @Test + void testAliasesWithoutDfmShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFormWeakAlias = TForm;") + .appendDecl(" TFrameWeakAlias = TFrame;") + .appendDecl(" TFormStrongAlias = type TForm;") + .appendDecl(" TFrameStrongAlias = type TFrame;")) + .verifyNoIssues(); + } + + @Test + void testFormWithDfmShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TForm)") + .appendDecl(" end;") + .appendImpl("{$R Foo.dfm}")) + .verifyNoIssues(); + } + + @Test + void testFrameWithDfmShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TFrame)") + .appendDecl(" end;") + .appendImpl("{$R Foo.dfm}")) + .verifyNoIssues(); + } + + @Test + void testFormWithoutDfmShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TForm) // Noncompliant") + .appendDecl(" end;")) + .verifyIssues(); + } + + @Test + void testFrameWithoutDfmShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TFrame) // Noncompliant") + .appendDecl(" end;")) + .verifyIssues(); + } + + @Test + void testNonDfmResourceShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TForm) // Noncompliant") + .appendDecl(" end;") + .appendImpl("{$R Foo.bar}")) + .verifyIssues(); + } + + @Test + void testNonDfmResourceContainingDotDfmShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TForm) // Noncompliant") + .appendDecl(" end;") + .appendImpl("{$R Foo.dfm.bar}")) + .verifyIssues(); + } + + @Test + void testIncludeDfmShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new FormDfmCheck()) + .withSearchPathUnit(createVclForms()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses") + .appendDecl(" Vcl.Forms;") + .appendDecl("type") + .appendDecl(" TFoo = class(TForm) // Noncompliant") + .appendDecl(" end;") + .appendImpl("{$I Foo.dfm}")) + .verifyIssues(); + } + + private static DelphiTestUnitBuilder createVclForms() { + return new DelphiTestUnitBuilder() + .unitName("Vcl.Forms") + .appendDecl("type") + .appendDecl(" TForm = class") + .appendDecl(" end;") + .appendDecl(" TFrame = class") + .appendDecl(" end;"); + } +}