Skip to content

Commit

Permalink
Implement FormDfm check
Browse files Browse the repository at this point in the history
  • Loading branch information
Cirras authored and fourls committed Jan 28, 2024
1 parent 919566b commit 92f9927
Show file tree
Hide file tree
Showing 7 changed files with 373 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public final class CheckList {
ForbiddenPropertyCheck.class,
ForbiddenRoutineCheck.class,
ForbiddenTypeCheck.class,
FormDfmCheck.class,
FreeAndNilTObjectCheck.class,
GotoStatementCheck.class,
GroupedFieldDeclarationCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<h2>Why is this an issue?</h2>
<p>
Omitting the corresponding <code>.dfm</code> 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.
</p>
<p>
Additionally, a unit that doesn't contain a <code>.dfm</code> resource will lack design-time form
editing functionality.
</p>
<p>
This rule flags any <code>TForm</code> or <code>TFrame</code> descendant in a unit that doesn't
include a <code>.dfm</code> resource.
</p>
<h2>How to fix it</h2>
<p>Add a <code>.dfm</code> resource to the unit:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
unit Foo;

interface

uses
Vcl.Forms;

type
TFoo = class(TForm)
// ...
end;

implementation

end.
</pre>
<pre data-diff-id="1" data-diff-type="compliant">
unit Foo;

interface

uses
Vcl.Forms;

type
TFoo = class(TForm)
// ...
end;

implementation

{$R *.dfm}

end.
</pre>
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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;");
}
}

0 comments on commit 92f9927

Please sign in to comment.