Skip to content

Commit ba8b827

Browse files
jlaurjlaur
committed
Add check for Optional used as a field type
Signed-off-by: Jacob Laursen <[email protected]>
1 parent 6f1f61f commit ba8b827

File tree

8 files changed

+273
-0
lines changed

8 files changed

+273
-0
lines changed

‎custom-checks/checkstyle/src/main/java/org/openhab/tools/analysis/checkstyle/OptionalFieldCheck.java‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Copyright (c) 2010-2025 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.tools.analysis.checkstyle;
14+
15+
import java.util.ArrayList;
16+
import java.util.List;
17+
18+
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
19+
import com.puppycrawl.tools.checkstyle.api.DetailAST;
20+
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
21+
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
22+
23+
/**
24+
* Check that generates <b> WARNING </b> when {@link java.util.Optional} is used as a field type.
25+
*
26+
* @author Jacob Laursen - Initial contribution
27+
*/
28+
public class OptionalFieldCheck extends AbstractCheck {
29+
30+
private static final String WARNING_MESSAGE_OPTIONAL_FIELD_USAGE = "Avoid using Optional as a field type";
31+
32+
private boolean importedOptional = false;
33+
private boolean starImportJavaUtil = false;
34+
35+
@Override
36+
public int[] getDefaultTokens() {
37+
return getAcceptableTokens();
38+
}
39+
40+
@Override
41+
public int[] getAcceptableTokens() {
42+
return new int[] { TokenTypes.IMPORT, TokenTypes.VARIABLE_DEF };
43+
}
44+
45+
@Override
46+
public int[] getRequiredTokens() {
47+
return CommonUtil.EMPTY_INT_ARRAY;
48+
}
49+
50+
@Override
51+
public void beginTree(DetailAST rootAST) {
52+
importedOptional = false;
53+
starImportJavaUtil = false;
54+
}
55+
56+
@Override
57+
public void visitToken(DetailAST ast) {
58+
switch (ast.getType()) {
59+
case TokenTypes.IMPORT:
60+
handleImport(ast);
61+
break;
62+
case TokenTypes.VARIABLE_DEF:
63+
handleVariableDef(ast);
64+
break;
65+
default:
66+
// No action
67+
}
68+
}
69+
70+
private void handleImport(DetailAST ast) {
71+
DetailAST dot = ast.findFirstToken(TokenTypes.DOT);
72+
if (dot == null) {
73+
return;
74+
}
75+
76+
String importText = flattenName(dot);
77+
if ("java.util.Optional".equals(importText)) {
78+
importedOptional = true;
79+
} else if ("java.util.*".equals(importText)) {
80+
starImportJavaUtil = true;
81+
}
82+
}
83+
84+
private void handleVariableDef(DetailAST ast) {
85+
// Ensure this is a field (not a local variable in a method)
86+
if (!isField(ast)) {
87+
return;
88+
}
89+
90+
DetailAST typeAst = ast.findFirstToken(TokenTypes.TYPE);
91+
if (typeAst == null) {
92+
return;
93+
}
94+
95+
DetailAST firstChild = typeAst.getFirstChild();
96+
if (firstChild == null) {
97+
return;
98+
}
99+
100+
String typeName = flattenName(firstChild);
101+
102+
// Fully qualified
103+
if ("java.util.Optional".equals(typeName)) {
104+
log(ast, WARNING_MESSAGE_OPTIONAL_FIELD_USAGE);
105+
return;
106+
}
107+
108+
// Simple name with import
109+
if ("Optional".equals(typeName) && (importedOptional || starImportJavaUtil)) {
110+
log(ast, WARNING_MESSAGE_OPTIONAL_FIELD_USAGE);
111+
}
112+
}
113+
114+
private boolean isField(DetailAST variableDefAst) {
115+
DetailAST parent = variableDefAst.getParent();
116+
return parent != null && parent.getType() == TokenTypes.OBJBLOCK && parent.getParent() != null
117+
&& parent.getParent().getType() == TokenTypes.CLASS_DEF;
118+
}
119+
120+
/**
121+
* Flatten a DOT/IDENT/STAR subtree into a dotted name, collecting only identifier parts and '*'.
122+
*
123+
* Examples:
124+
* - IDENT -> "Optional"
125+
* - DOT(java, util, Optional, TYPE_ARGUMENTS) -> "java.util.Optional"
126+
* - DOT(java, util, STAR) -> "java.util.*"
127+
*
128+
* Example tree for fully qualified:
129+
*
130+
* <pre>
131+
* --DOT -> .
132+
* |--DOT -> .
133+
* |--IDENT -> java
134+
* |--IDENT -> util
135+
* |--IDENT -> Optional
136+
* |--ARGUMENTS
137+
* </pre>
138+
*/
139+
private String flattenName(DetailAST node) {
140+
List<String> parts = new ArrayList<>();
141+
collectNameParts(node, parts);
142+
if (parts.isEmpty()) {
143+
return "";
144+
}
145+
return String.join(".", parts);
146+
}
147+
148+
private void collectNameParts(DetailAST node, List<String> parts) {
149+
if (node == null) {
150+
return;
151+
}
152+
153+
final int type = node.getType();
154+
155+
if (type == TokenTypes.IDENT) {
156+
parts.add(node.getText());
157+
return;
158+
}
159+
160+
if (type == TokenTypes.STAR) {
161+
// Wildcard import
162+
parts.add("*");
163+
return;
164+
}
165+
166+
if (type == TokenTypes.DOT) {
167+
// Iterate children in order and collect IDENT / DOT / STAR, ignore TYPE_ARGUMENTS, etc.
168+
DetailAST child = node.getFirstChild();
169+
while (child != null) {
170+
final int childType = child.getType();
171+
if (childType == TokenTypes.IDENT) {
172+
parts.add(child.getText());
173+
} else if (childType == TokenTypes.DOT) {
174+
// Recurse to expand nested package parts
175+
collectNameParts(child, parts);
176+
} else if (childType == TokenTypes.STAR) {
177+
parts.add("*");
178+
} // Else ignore TYPE_ARGUMENTS, TYPE_PARAMETERS, etc.
179+
child = child.getNextSibling();
180+
}
181+
}
182+
}
183+
}

‎custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/OptionalFieldCheckTest.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright (c) 2010-2025 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.tools.analysis.checkstyle.test;
14+
15+
import org.junit.jupiter.api.Test;
16+
import org.openhab.tools.analysis.checkstyle.OptionalFieldCheck;
17+
import org.openhab.tools.analysis.checkstyle.api.AbstractStaticCheckTest;
18+
19+
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
20+
21+
/**
22+
* Tests for {@link OptionalFieldCheck}
23+
*
24+
* @author Jacob Laursen - Initial contribution
25+
*/
26+
public class OptionalFieldCheckTest extends AbstractStaticCheckTest {
27+
28+
@Override
29+
protected String getPackageLocation() {
30+
return "checkstyle/optionalFieldCheckTest";
31+
}
32+
33+
@Test
34+
public void testOptionalFieldFromImportDetected() throws Exception {
35+
final String[] expected = { "4:5: Avoid using Optional as a field type" };
36+
verify(createModuleConfig(OptionalFieldCheck.class), getPath("OptionalFieldFromImport.java"), expected);
37+
}
38+
39+
@Test
40+
public void testOptionalFieldFromStarImportDetected() throws Exception {
41+
final String[] expected = { "4:5: Avoid using Optional as a field type" };
42+
verify(createModuleConfig(OptionalFieldCheck.class), getPath("OptionalFieldFromStarImport.java"), expected);
43+
}
44+
45+
@Test
46+
public void testOptionalFieldFullyQualifiedDetected() throws Exception {
47+
final String[] expected = { "2:5: Avoid using Optional as a field type" };
48+
verify(createModuleConfig(OptionalFieldCheck.class), getPath("OptionalFieldFullyQualified.java"), expected);
49+
}
50+
51+
@Test
52+
public void testOptionalFieldFromImportOtherPackageNotDetected() throws Exception {
53+
verify(createModuleConfig(OptionalFieldCheck.class), getPath("OptionalFieldFromImportOtherPackage.java"),
54+
CommonUtil.EMPTY_STRING_ARRAY);
55+
}
56+
57+
@Test
58+
public void testOptionalLocalVariableNotDetected() throws Exception {
59+
verify(createModuleConfig(OptionalFieldCheck.class), getPath("OptionalLocalVariable.java"),
60+
CommonUtil.EMPTY_STRING_ARRAY);
61+
}
62+
}

‎custom-checks/checkstyle/src/test/resources/checkstyle/optionalFieldCheckTest/OptionalFieldFromImport.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import java.util.Optional;
2+
3+
public class OptionalFieldFromImport {
4+
private Optional<String> stringOrNoString;
5+
}

‎custom-checks/checkstyle/src/test/resources/checkstyle/optionalFieldCheckTest/OptionalFieldFromImportOtherPackage.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import org.openhab.Optional;
2+
3+
public class OptionalFieldFromImportOtherPackage {
4+
private Optional<String> stringOrNoString;
5+
}

‎custom-checks/checkstyle/src/test/resources/checkstyle/optionalFieldCheckTest/OptionalFieldFromStarImport.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import java.util.*;
2+
3+
public class OptionalFieldFromImport {
4+
private Optional<String> stringOrNoString;
5+
}

‎custom-checks/checkstyle/src/test/resources/checkstyle/optionalFieldCheckTest/OptionalFieldFullyQualified.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
public class OptionalFieldFullyQualified {
2+
private java.util.Optional<String> stringOrNoString;
3+
}

‎custom-checks/checkstyle/src/test/resources/checkstyle/optionalFieldCheckTest/OptionalLocalVariable.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import java.util.Optional;
2+
3+
public class OptionalLocalVariable {
4+
private void test() {
5+
Optional<String> stringOrNoString;
6+
}
7+
}

‎sat-plugin/src/main/resources/rulesets/checkstyle/rules.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@
143143
<property name="forbiddenPackages" value="${checkstyle.forbiddenPackageUsageCheck.forbiddenPackages}"/>
144144
<property name="exceptions" value="${checkstyle.forbiddenPackageUsageCheck.exceptions}"/>
145145
</module>
146+
<module name="org.openhab.tools.analysis.checkstyle.OptionalFieldCheck">
147+
<property name="severity" value="warning"/>
148+
</module>
146149
<module name="UnusedImports">
147150
<property name="processJavadoc" value="true"/>
148151
<property name="severity" value="info"/>

0 commit comments

Comments
 (0)