Skip to content

Commit

Permalink
fix empty comment detection for large comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Luro02 committed Jun 30, 2024
1 parent 13b427c commit fc78f7e
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,77 @@
import de.firemage.autograder.core.integrated.StaticAnalysis;
import spoon.processing.AbstractProcessor;
import spoon.reflect.code.CtComment;
import spoon.reflect.code.CtStatement;
import spoon.reflect.declaration.CtElement;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

@ExecutableCheck(reportedProblems = { ProblemType.UNNECESSARY_COMMENT })
public class UnnecessaryComment extends IntegratedCheck {
private final Set<CtComment> visitedComments = Collections.newSetFromMap(new IdentityHashMap<>());

private void checkComments(Collection<? extends CtComment> comments) {
this.visitedComments.addAll(comments);

if (comments.size() != 1) {
return;
}

for (CtComment ctComment : comments) {
if (ctComment.getContent().isBlank()) {
addLocalProblem(
ctComment,
new LocalizedMessage("unnecessary-comment-empty"),
ProblemType.UNNECESSARY_COMMENT
);
break;
}
}
}

private static boolean isStandaloneComment(CtComment ctComment) {
return ctComment.getParent() instanceof CtElement ctElement && !ctElement.getComments().contains(ctComment);
}

@Override
protected void check(StaticAnalysis staticAnalysis) {
staticAnalysis.processWith(new AbstractProcessor<CtComment>() {
staticAnalysis.processWith(new AbstractProcessor<>() {
@Override
public void process(CtComment ctComment) {
if (ctComment.isImplicit()
|| !ctComment.getPosition().isValidPosition()
|| ctComment.getCommentType() != CtComment.CommentType.INLINE) {
public void process(CtElement element) {
if (element.isImplicit() || !element.getPosition().isValidPosition()) {
return;
}

CtStatement previous = SpoonUtil.getPreviousStatement(ctComment).orElse(null);
if (previous instanceof CtComment) {
// The main problem is that one can use empty comments as spacers in larger comments.
//
// ^ like this.
//
// And to detect this correctly, we need to check the context of the comment as well
// (if there are other comments around it).

// Comments can appear either attached to a CtElement or as a standalone comment.
//
// Here the comments without an attached CtElement are processed:
if (element instanceof CtComment ctComment && isStandaloneComment(ctComment) && !visitedComments.contains(ctComment)) {
List<CtComment> followingComments = SpoonUtil.getNextStatements(ctComment)
.stream()
.takeWhile(CtComment.class::isInstance)
.map(CtComment.class::cast)
.collect(Collectors.toCollection(ArrayList::new));

followingComments.addFirst(ctComment);

checkComments(followingComments);
return;
}

if (ctComment.getContent().isBlank()) {
addLocalProblem(
ctComment,
new LocalizedMessage("unnecessary-comment-empty"),
ProblemType.UNNECESSARY_COMMENT
);
if (!element.getComments().isEmpty()) {
checkComments(element.getComments());
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
class TestUnnecessaryComment extends AbstractCheckTest {
private static final List<ProblemType> PROBLEM_TYPES = List.of(ProblemType.UNNECESSARY_COMMENT);

void assertEqualsEmpy(Problem problem) {
void assertEqualsEmpty(Problem problem) {
assertEquals(
this.linter.translateMessage(new LocalizedMessage("unnecessary-comment-empty")),
this.linter.translateMessage(problem.getExplanation())
Expand All @@ -41,7 +41,7 @@ public Test() {
PROBLEM_TYPES
);

assertEqualsEmpy(problems.next());
assertEqualsEmpty(problems.next());

problems.assertExhausted();
}
Expand All @@ -67,4 +67,38 @@ public Test() {

problems.assertExhausted();
}

@Test
void testVeryLongComment() throws IOException, LinterException {
ProblemIterator problems = this.checkIterator(
StringSourceInfo.fromSourceString(
JavaVersion.JAVA_17,
"Test",
"""
public class Test {
public Test() {
// One of the difficulties of this task is parsing the command line arguments correctly.
// In many assignments the commands are given as a single word like "jaccard" or "quit"
// and commands with multiple words would be joined by an underscore like "add_author" or
// "publications-by".
//
// In this assignment some commands like "add author" or "publications by" contain spaces
// and not all arguments are separated by space like "add journal <journal>,<publisher>".
//
// This makes splitting the input by a single character (like a space) difficult, because for example
// > add journal Science Direct,Elsevier
// is a valid input and should be parsed into ["add journal", "Science Direct", "Elsevier"]
//
// To solve this problem, we check if our input starts with the command name (for example "add author")
// and we then remove the command name from the input and pass the rest as arguments to the command.
String commandWithArguments = "add author John Doe";
}
}
"""
),
PROBLEM_TYPES
);

problems.assertExhausted();
}
}

0 comments on commit fc78f7e

Please sign in to comment.