Skip to content

Commit

Permalink
Add checks for redundant self usage
Browse files Browse the repository at this point in the history
  • Loading branch information
adityatrivedi committed Dec 17, 2015
1 parent 95e8cb3 commit 4505848
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/main/antlr/com/sleekbyte/tailor/antlr/Swift.g4
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ functionName : identifier | operator ;
functionSignature : parameterClauses ('throws' | 'rethrows')? functionResult? ;
functionResult : '->' attributes? sType ;
functionBody : codeBlock ;
parameterClauses : parameterClause parameterClauses? ;
parameterClauses : parameterClause+ ;
parameterClause : '(' ')' | '(' parameterList '...'? ')' ;
parameterList : parameter | parameter ',' parameterList ;
// Parameters don't have attributes in the Swift Language Reference
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/sleekbyte/tailor/common/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ public class Messages {
+ " or <TODO(dev-name): description>";
public static final String REDUNDANT_METHOD_PARENTHESES = "following method call with trailing closure argument"
+ " should be removed";
public static final String EXPLICIT_CALL_TO_SELF = "References to self should only be made in closures and to "
+ "prevent parameter name conflicts.";

// Usage messages
public static final String CMD_LINE_SYNTAX = "tailor [options] [--] [[file|directory] ...]";
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/sleekbyte/tailor/common/Rules.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.sleekbyte.tailor.listeners.LowerCamelCaseListener;
import com.sleekbyte.tailor.listeners.MultipleImportListener;
import com.sleekbyte.tailor.listeners.RedundantParenthesesListener;
import com.sleekbyte.tailor.listeners.RedundantSelfListener;
import com.sleekbyte.tailor.listeners.SemicolonTerminatedListener;
import com.sleekbyte.tailor.listeners.TodoCommentListener;
import com.sleekbyte.tailor.listeners.UpperCamelCaseListener;
Expand Down Expand Up @@ -45,6 +46,7 @@ public enum Rules {
MULTIPLE_IMPORTS,
OPERATOR_WHITESPACE,
REDUNDANT_PARENTHESES,
REDUNDANT_SELF,
TERMINATING_NEWLINE,
TERMINATING_SEMICOLON,
TODO_SYNTAX,
Expand Down Expand Up @@ -165,6 +167,10 @@ public String getLink() {
+ "values assigned in variable/constant declarations should not be enclosed in parentheses.";
REDUNDANT_PARENTHESES.className = RedundantParenthesesListener.class.getName();

REDUNDANT_SELF.name = "redundant-self";
REDUNDANT_SELF.description = "Redundant self.";
REDUNDANT_SELF.className = RedundantSelfListener.class.getName();

TERMINATING_NEWLINE.name = "terminating-newline";
TERMINATING_NEWLINE.description = "Verify that source files terminate with exactly one '\\n' character.";
TERMINATING_NEWLINE.className = FileListener.class.getName();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package com.sleekbyte.tailor.listeners;

import com.sleekbyte.tailor.antlr.SwiftBaseListener;
import com.sleekbyte.tailor.antlr.SwiftParser;
import com.sleekbyte.tailor.antlr.SwiftParser.ClosureExpressionContext;
import com.sleekbyte.tailor.antlr.SwiftParser.ExternalParameterNameContext;
import com.sleekbyte.tailor.antlr.SwiftParser.FunctionDeclarationContext;
import com.sleekbyte.tailor.antlr.SwiftParser.InitializerDeclarationContext;
import com.sleekbyte.tailor.antlr.SwiftParser.LocalParameterNameContext;
import com.sleekbyte.tailor.antlr.SwiftParser.ParameterClauseContext;
import com.sleekbyte.tailor.antlr.SwiftParser.ParameterContext;
import com.sleekbyte.tailor.antlr.SwiftParser.ParameterListContext;
import com.sleekbyte.tailor.common.Location;
import com.sleekbyte.tailor.common.Messages;
import com.sleekbyte.tailor.common.Rules;
import com.sleekbyte.tailor.output.Printer;
import com.sleekbyte.tailor.utils.ListenerUtil;
import com.sleekbyte.tailor.utils.ParseTreeUtil;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.tree.ParseTree;

import java.util.ArrayList;
import java.util.List;

/**
* Parse tree listener for redundant self keyword usage.
*/
public final class RedundantSelfListener extends SwiftBaseListener {

private Printer printer;

public RedundantSelfListener(Printer printer) {
this.printer = printer;
}

@Override
public void enterSelfExpression(SwiftParser.SelfExpressionContext ctx) {
Location location = ListenerUtil.getContextStartLocation(ctx);
// Do not flag
// 1. standalone self usages
// 2. self usage in initializer(s) and closures
ParseTree dot = ParseTreeUtil.getRightNode(ctx);
if (dot != null && !isInInitializerOrClosure(ctx)) {
// Extract function parameter names
List<String> parameterNames = getFunctionParameters(getParentFunction(ctx));
ParseTree property = ParseTreeUtil.getRightSibling(dot);
if (property != null && parameterNames.contains(property.getText())) {
return;
}
// Flag usage of self
// 1. outside closures and initializer(s)
// 2. inside methods that do not have same named parameters
printer.warn(Rules.REDUNDANT_SELF, Messages.EXPLICIT_CALL_TO_SELF, location);

}
}

private static boolean isInInitializerOrClosure(ParserRuleContext ctx) {
if (ctx == null) {
return false;
}
while (ctx != null) {
ctx = ctx.getParent();
if (ctx instanceof ClosureExpressionContext
|| ctx instanceof InitializerDeclarationContext) {
return true;
}
}
return false;
}

private static FunctionDeclarationContext getParentFunction(ParserRuleContext ctx) {
if (ctx == null) {
return null;
}
while (ctx != null) {
ctx = ctx.getParent();
if (ctx instanceof FunctionDeclarationContext) {
return (FunctionDeclarationContext) ctx;
}
}
return null;
}

private List<String> getFunctionParameters(FunctionDeclarationContext ctx) {
ArrayList<String> parameterNames = new ArrayList<>();

if (ctx == null) {
return parameterNames;
}

List<ParameterClauseContext> parameterClauses = ctx.functionSignature().parameterClauses().parameterClause();
for (ParameterClauseContext parameterClause : parameterClauses) {
ParameterListContext parameterList = parameterClause.parameterList();
if (parameterList != null) {
for (ParseTree item : parameterList.children) {
ParameterContext parameter;
if (item.getText().equals(",")) {
continue;
} else if (item instanceof ParameterListContext) {
parameter = ((ParameterListContext) item).parameter();
} else {
parameter = (ParameterContext) item;
}
LocalParameterNameContext localParameterName = parameter.localParameterName();
if (localParameterName != null) {
parameterNames.add(parameter.localParameterName().getText());
} else {
ExternalParameterNameContext externalParameterName = parameter.externalParameterName();
parameterNames.add(externalParameterName.getText());
}
}
}
}
return parameterNames;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNodeImpl;


/**
* Utils for traversing Parse Trees.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.sleekbyte.tailor.functional;


import com.sleekbyte.tailor.common.Messages;
import com.sleekbyte.tailor.common.Rules;
import com.sleekbyte.tailor.common.Severity;
import com.sleekbyte.tailor.output.Printer;
import org.junit.runner.RunWith;
import org.mockito.runners.MockitoJUnitRunner;

/**
* Functional tests for {@link com.sleekbyte.tailor.listeners.RedundantSelfListener}.
*/
@RunWith(MockitoJUnitRunner.class)
public final class RedundantSelfTest extends RuleTest {

@Override
protected String[] getCommandArgs() {
return new String[] {
"--only", "redundant-self"
};
}

@Override
protected void addAllExpectedMsgs() {
addExpectedMsg(13, 12);
addExpectedMsg(14, 19);
addExpectedMsg(22, 9);
addExpectedMsg(22, 19);
addExpectedMsg(35, 9);
}

private void addExpectedMsg(int line, int column) {
expectedMessages.add(Printer.genOutputStringForTest(Rules.REDUNDANT_SELF, inputFile.getName(), line, column,
Severity.WARNING, Messages.EXPLICIT_CALL_TO_SELF));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import Foundation

class SomeClass {
let x = 2
let y = 10

init() {
print(self.x)
self.demo(2, y: "2")
}

func demo(d: Int, y: String) {
if self.x == 2 {
print(self.x)
} else {
print(self.y)
}
}

func callDemoTwice() {
demo(x, y: "2")
self.demo(self.x, y: "2")
}

}

private class History {
var events: [String]

init(events: [String]) {
self.events = events
}

func rewrite() {
self.events = []
}

}

extension History {

var whenVictorious: () -> () {
return {
self.rewrite()
}
}
}

0 comments on commit 4505848

Please sign in to comment.