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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
void main() {
int a = 5;
int x = 10; // Noncompliant {{Rename "x" which hides the field declared at line 11.}}
// ^
float b = 3.14f;
System.out.println(a);
System.out.println(x);
System.out.println(b);
}

int x = 20;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// SONARJAVA-6028: FPs ahead. Only the line with "Too much." should be noncompliant.

void main() { // Noncompliant
System.out.println("Just right."); // Noncompliant
if (true) {
System.out.println("Too much."); // Noncompliant
}
}

class MyClass { // Noncompliant
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public List<Tree.Kind> nodesToVisit() {
Tree.Kind.INTERFACE,
Tree.Kind.ANNOTATION_TYPE,
Tree.Kind.RECORD,
Tree.Kind.IMPLICIT_CLASS,
Tree.Kind.VARIABLE,
Tree.Kind.METHOD,
Tree.Kind.CONSTRUCTOR,
Expand Down Expand Up @@ -130,7 +131,14 @@ private static boolean isInStaticInnerClass(VariableTree hiddenVariable, Variabl
}

private static boolean isClassTree(Tree tree) {
return tree.is(Tree.Kind.CLASS, Tree.Kind.ENUM, Tree.Kind.INTERFACE, Tree.Kind.ANNOTATION_TYPE, Tree.Kind.RECORD);
return tree.is(
Tree.Kind.CLASS,
Tree.Kind.ENUM,
Tree.Kind.INTERFACE,
Tree.Kind.ANNOTATION_TYPE,
Tree.Kind.RECORD,
Tree.Kind.IMPLICIT_CLASS
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ public void scanFile(JavaFileScannerContext context) {

@Override
public void visitClass(ClassTree tree) {
// Exclude anonymous classes
boolean isAnonymous = tree.simpleName() == null;
if (!isAnonymous) {
// Exclude anonymous classes other than implicit classed of compact source files.
boolean isExcluded = tree.simpleName() == null && !tree.is(Kind.IMPLICIT_CLASS);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to address this but I wonder in how many places we infer a class is anonymous from it simpleName is null rather than using a more explicit Kind

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!isExcluded) {
checkIndentation(Collections.singletonList(tree));
}
int previousLevel = expectedLevel;
if (isAnonymous) {
if (isExcluded) {
excludeIssueAtLine = LineUtils.startLine(tree.openBraceToken());
expectedLevel = Position.startOf(tree.closeBraceToken()).columnOffset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,11 @@ void test_records() {
.verifyIssues();
}

@Test
void test_compact_source() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/HiddenFieldCheckCompactSample.java"))
.withCheck(new HiddenFieldCheck())
.verifyIssues();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,12 @@ void tolerates_line_breaking_control_characters() {
.withCheck(new IndentationCheck())
.verifyNoIssues();
}

@Test
void compact_source_file() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/IndentationCheck_compactSource.java"))
.withCheck(new IndentationCheck())
.verifyIssues();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public void scanFile(JavaFileScannerContext context) {
currentClassKey.clear();
parent.clear();
anonymousInnerClassCounter.clear();
// 0 stored on the top will be used for implicit anonymous classes in compact source files.
anonymousInnerClassCounter.push(0);
scan(tree);
}

Expand Down Expand Up @@ -77,9 +79,10 @@ private String getClassKey(String className) {
key = currentPackage + "/" + className;
}
if ("".equals(className) || (parent.peek() != null && parent.peek().is(Tree.Kind.METHOD))) {
// inner class declared within method
// inner class declared within method or implicitly declared class in a compact source file
int count = anonymousInnerClassCounter.pop() + 1;
key = currentClassKey.peek() + "$" + count + className;
String prefix = currentClassKey.isEmpty() ? "" : currentClassKey.peek();
key = prefix + "$" + count + className;
anonymousInnerClassCounter.push(count);
} else if (currentClassKey.peek() != null) {
key = currentClassKey.peek() + "$" + className;
Expand Down
22 changes: 18 additions & 4 deletions java-frontend/src/main/java/org/sonar/java/Measurer.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.sonar.java;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.LinkedList;
Expand All @@ -38,6 +39,15 @@

public class Measurer extends SubscriptionVisitor {

private static final Tree.Kind[] CLASS_KINDS = new Tree.Kind[]{
Tree.Kind.CLASS,
Tree.Kind.INTERFACE,
Tree.Kind.ENUM,
Tree.Kind.ANNOTATION_TYPE,
Tree.Kind.RECORD,
Tree.Kind.IMPLICIT_CLASS
};

private final SensorContext sensorContext;
private final NoSonarFilter noSonarFilter;
private InputFile sonarFile;
Expand All @@ -61,9 +71,13 @@ public void scanFile(JavaFileScannerContext context) {

@Override
public List<Tree.Kind> nodesToVisit() {
return Arrays.asList(Tree.Kind.CLASS, Tree.Kind.INTERFACE, Tree.Kind.ENUM, Tree.Kind.ANNOTATION_TYPE, Tree.Kind.RECORD,
Tree.Kind.NEW_CLASS, Tree.Kind.ENUM_CONSTANT,
Tree.Kind.METHOD, Tree.Kind.CONSTRUCTOR);
List<Tree.Kind> nodes = new ArrayList<>(Arrays.asList(CLASS_KINDS));
nodes.addAll(Arrays.asList(
Tree.Kind.NEW_CLASS,
Tree.Kind.ENUM_CONSTANT,
Tree.Kind.METHOD,
Tree.Kind.CONSTRUCTOR));
return nodes;
}


Expand Down Expand Up @@ -120,7 +134,7 @@ public void leaveNode(Tree tree) {
}

private static boolean isClassTree(Tree tree) {
return tree.is(Tree.Kind.CLASS, Tree.Kind.INTERFACE, Tree.Kind.ENUM, Tree.Kind.ANNOTATION_TYPE, Tree.Kind.RECORD);
return tree.is(CLASS_KINDS);
}

private <T extends Serializable> void saveMetricOnFile(Metric<T> metric, T value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ private PublicApiChecker() {
Tree.Kind.INTERFACE,
Tree.Kind.ENUM,
Tree.Kind.ANNOTATION_TYPE,
Tree.Kind.RECORD
Tree.Kind.RECORD,
Tree.Kind.IMPLICIT_CLASS
};

private static final Tree.Kind[] METHOD_KINDS = {
Expand Down
25 changes: 23 additions & 2 deletions java-frontend/src/main/java/org/sonar/java/model/JParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,27 @@ private ModuleDirectiveTree convertModuleDirective(ModuleDirective node) {
}
}

/** Converts implicitly declared unnamed class at the top level of a compact compilation unit. */
private ClassTreeImpl convertUnnamedClassDeclaration(AbstractTypeDeclaration e) {
List<Tree> members = new ArrayList<>();

for (Object o : e.bodyDeclarations()) {
processBodyDeclaration((BodyDeclaration) o, members);
}

ClassTreeImpl t = new ClassTreeImpl(Tree.Kind.IMPLICIT_CLASS, null, members, null);

t.typeBinding = e.resolveBinding();
declaration(t.typeBinding, t);

return t;
}

private ClassTreeImpl convertTypeDeclaration(AbstractTypeDeclaration e) {
if (e.getNodeType() == ASTNode.UNNAMED_CLASS) {
return convertUnnamedClassDeclaration(e);
}

List<Tree> members = new ArrayList<>();

int leftBraceTokenIndex = findLeftBraceTokenIndex(e);
Expand Down Expand Up @@ -932,7 +952,7 @@ private static List<?> superInterfaceTypes(AbstractTypeDeclaration e) {
return ((EnumDeclaration) e).superInterfaceTypes();
case ASTNode.RECORD_DECLARATION:
return ((RecordDeclaration) e).superInterfaceTypes();
case ASTNode.ANNOTATION_TYPE_DECLARATION:
case ASTNode.ANNOTATION_TYPE_DECLARATION, ASTNode.UNNAMED_CLASS:
return Collections.emptyList();
default:
throw new IllegalStateException(ASTNode.nodeClassForType(e.getNodeType()).toString());
Expand Down Expand Up @@ -1021,7 +1041,8 @@ private void processBodyDeclaration(BodyDeclaration node, List<Tree> members) {
case ASTNode.ANNOTATION_TYPE_DECLARATION,
ASTNode.ENUM_DECLARATION,
ASTNode.RECORD_DECLARATION,
ASTNode.TYPE_DECLARATION:
ASTNode.TYPE_DECLARATION,
ASTNode.UNNAMED_CLASS:
lastTokenIndex = processTypeDeclaration((AbstractTypeDeclaration) node, members);
break;
case ASTNode.ANNOTATION_TYPE_MEMBER_DECLARATION:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@
public class ClassTreeImpl extends JavaTree implements ClassTree {

private final Kind kind;
@Nullable
private final SyntaxToken openBraceToken;
private final List<Tree> members;
@Nullable
private final SyntaxToken closeBraceToken;
private ModifiersTree modifiers;
private SyntaxToken atToken;
Expand All @@ -69,7 +71,7 @@ public class ClassTreeImpl extends JavaTree implements ClassTree {
@Nullable
public ITypeBinding typeBinding;

public ClassTreeImpl(Kind kind, SyntaxToken openBraceToken, List<Tree> members, SyntaxToken closeBraceToken) {
public ClassTreeImpl(Kind kind, @Nullable SyntaxToken openBraceToken, List<Tree> members, @Nullable SyntaxToken closeBraceToken) {
this.kind = kind;
this.openBraceToken = openBraceToken;
this.members = orderMembers(kind, members);
Expand Down Expand Up @@ -197,6 +199,7 @@ public ListTree<TypeTree> permittedTypes() {
return permittedTypes;
}

@Nullable
@Override
public SyntaxToken openBraceToken() {
return openBraceToken;
Expand All @@ -207,6 +210,7 @@ public List<Tree> members() {
return members;
}

@Nullable
@Override
public SyntaxToken closeBraceToken() {
return closeBraceToken;
Expand Down Expand Up @@ -262,9 +266,9 @@ public List<Tree> children() {
addIfNotNull(implementsKeyword),
Collections.singletonList(superInterfaces),
Collections.singletonList(permittedTypes),
Collections.singletonList(openBraceToken),
addIfNotNull(openBraceToken),
members,
Collections.singletonList(closeBraceToken)
addIfNotNull(closeBraceToken)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ enum Kind implements GrammarRuleKey {
*/
RECORD(ClassTree.class),

/**
* {@link ClassTree}
*
* @since Java 25
*/
IMPLICIT_CLASS(ClassTree.class),

/**
* {@link EnumConstantTree}
*
Expand Down
10 changes: 10 additions & 0 deletions java-frontend/src/test/files/metrics/CompactSource.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
void main() {
int a = 5;
int x = 10;
float b = 3.14f;
System.out.println(a);
System.out.println(x);
System.out.println(b);
}

int x = 20;
10 changes: 10 additions & 0 deletions java-frontend/src/test/java/org/sonar/java/JavaFilesCacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,14 @@ void resource_file_mapping() {
"org/sonar/java/JavaFilesCacheTestFile$A$3");
}

@Test
void compact_source() {
JavaFilesCache javaFilesCache = new JavaFilesCache();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about JavaFilesCache

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

JavaAstScanner.scanSingleFileForTests(TestUtils.inputFile("src/test/resources/JavaFilesCacheTestFileCompact.java"), new VisitorsBridge(javaFilesCache));

Set<String> classNames = javaFilesCache.getClassNames();
assertThat(classNames)
.hasSize(4)
.contains("$1$1", "$1", "$1$C", "$1$C$D");
}
}
5 changes: 5 additions & 0 deletions java-frontend/src/test/java/org/sonar/java/MeasurerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ void verify_statements_metric() {
checkMetric("Statements.java", "statements", 18);
}

@Test
void verify_compact_source() {
checkMetric("CompactSource.java", "classes", 1);
}

@Test
void verify_ncloc_metric() {
checkMetric("LinesOfCode.java", "ncloc", 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ void private_constructor() throws Exception {
@Test
void targeted_kinds() {
assertThat(PublicApiChecker.classKinds())
.hasSize(5)
.containsExactlyInAnyOrder(Tree.Kind.ANNOTATION_TYPE, Tree.Kind.ENUM, Tree.Kind.CLASS, Tree.Kind.INTERFACE, Tree.Kind.RECORD);
.hasSize(6)
.containsExactlyInAnyOrder(Tree.Kind.ANNOTATION_TYPE, Tree.Kind.ENUM, Tree.Kind.CLASS, Tree.Kind.INTERFACE, Tree.Kind.RECORD, Tree.Kind.IMPLICIT_CLASS);

assertThat(PublicApiChecker.methodKinds())
.hasSize(2)
.containsExactlyInAnyOrder(Tree.Kind.CONSTRUCTOR, Tree.Kind.METHOD);

assertThat(PublicApiChecker.apiKinds())
.hasSize(8)
.hasSize(9)
.contains(PublicApiChecker.classKinds())
.contains(PublicApiChecker.methodKinds())
.contains(Tree.Kind.VARIABLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2102,4 +2102,43 @@ public class Source {
JavaTree.ImportTreeImpl importConst = (JavaTree.ImportTreeImpl) cu.imports().get(1);
assertThat(importConst.isModule()).isFalse();
}

@Test
void compactSource_simple() {
String source = """
void main() {
}
""";
JavaTree.CompilationUnitTreeImpl cu = test(source);
assertThat(cu.types()).hasSize(1);
ClassTreeImpl clazz = (ClassTreeImpl) cu.types().get(0);
assertThat(clazz).isNotNull();
assertThat(clazz.kind()).isEqualTo(Tree.Kind.IMPLICIT_CLASS);
assertThat(clazz.simpleName()).isNull();
assertThat(clazz.openBraceToken()).isNull();
assertThat(clazz.closeBraceToken()).isNull();
}

@Test
void compactSource_complex() {
String source = """
void main() {
System.out.println("Hello, World!");
}
int i = 43;
class Helper {
void help() {
System.out.println("Helping...");
}
}
""";
JavaTree.CompilationUnitTreeImpl cu = test(source);
ClassTreeImpl clazz = (ClassTreeImpl) cu.types().get(0);
assertThat(clazz).isNotNull();
assertThat(clazz.kind()).isEqualTo(Tree.Kind.IMPLICIT_CLASS);
assertThat(clazz.members()).hasSize(3);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we explicitly test that the 3 members are a method, a variable and a type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

assertThat(clazz.members().get(0).kind()).isEqualTo(Tree.Kind.METHOD);
assertThat(clazz.members().get(1).kind()).isEqualTo(Tree.Kind.VARIABLE);
assertThat(clazz.members().get(2).kind()).isEqualTo(Tree.Kind.CLASS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class TreeTest {

@Test
void test() {
assertThat(Tree.Kind.values()).hasSize(128);
assertThat(Tree.Kind.values()).hasSize(129);
}

}
Loading
Loading