diff --git a/src/main/antlr/com/sleekbyte/tailor/antlr/Swift.g4 b/src/main/antlr/com/sleekbyte/tailor/antlr/Swift.g4 index dae96c22..f399a327 100644 --- a/src/main/antlr/com/sleekbyte/tailor/antlr/Swift.g4 +++ b/src/main/antlr/com/sleekbyte/tailor/antlr/Swift.g4 @@ -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 diff --git a/src/main/java/com/sleekbyte/tailor/common/Messages.java b/src/main/java/com/sleekbyte/tailor/common/Messages.java index 5a292bde..40d8bfb1 100644 --- a/src/main/java/com/sleekbyte/tailor/common/Messages.java +++ b/src/main/java/com/sleekbyte/tailor/common/Messages.java @@ -105,6 +105,8 @@ public class Messages { + " or "; 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] ...]"; diff --git a/src/main/java/com/sleekbyte/tailor/common/Rules.java b/src/main/java/com/sleekbyte/tailor/common/Rules.java index a3eb26ee..b3cf2286 100644 --- a/src/main/java/com/sleekbyte/tailor/common/Rules.java +++ b/src/main/java/com/sleekbyte/tailor/common/Rules.java @@ -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; @@ -45,6 +46,7 @@ public enum Rules { MULTIPLE_IMPORTS, OPERATOR_WHITESPACE, REDUNDANT_PARENTHESES, + REDUNDANT_SELF, TERMINATING_NEWLINE, TERMINATING_SEMICOLON, TODO_SYNTAX, @@ -165,6 +167,11 @@ 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 = "Only explicitly use the self keyword when required by the language i.e. in a " + + "closure, or when parameter names conflict."; + 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(); diff --git a/src/main/java/com/sleekbyte/tailor/listeners/RedundantSelfListener.java b/src/main/java/com/sleekbyte/tailor/listeners/RedundantSelfListener.java new file mode 100644 index 00000000..8f22b7c8 --- /dev/null +++ b/src/main/java/com/sleekbyte/tailor/listeners/RedundantSelfListener.java @@ -0,0 +1,115 @@ +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.DynamicTypeExpressionContext; +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 && dot.getText().equals(".") && !insideInitializerOrClosureOrDynamicType(ctx)) { + // Extract function parameter names + List 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 whose parameters don't have the same name as the property + printer.warn(Rules.REDUNDANT_SELF, Messages.EXPLICIT_CALL_TO_SELF, location); + + } + } + + private static boolean insideInitializerOrClosureOrDynamicType(ParserRuleContext ctx) { + if (ctx == null) { + return false; + } + while (ctx != null) { + ctx = ctx.getParent(); + if (ctx instanceof ClosureExpressionContext + || ctx instanceof InitializerDeclarationContext + || ctx instanceof DynamicTypeExpressionContext) { + 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 getFunctionParameters(FunctionDeclarationContext ctx) { + ArrayList parameterNames = new ArrayList<>(); + + if (ctx == null) { + return parameterNames; + } + + List 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()); + } + } + } + } + return parameterNames; + } +} diff --git a/src/test/java/com/sleekbyte/tailor/functional/RedundantSelfTest.java b/src/test/java/com/sleekbyte/tailor/functional/RedundantSelfTest.java new file mode 100644 index 00000000..d1a8c431 --- /dev/null +++ b/src/test/java/com/sleekbyte/tailor/functional/RedundantSelfTest.java @@ -0,0 +1,38 @@ +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, 15); + addExpectedMsg(27, 9); + addExpectedMsg(27, 19); + addExpectedMsg(40, 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)); + } +} diff --git a/src/test/swift/com/sleekbyte/tailor/functional/RedundantSelfTest.swift b/src/test/swift/com/sleekbyte/tailor/functional/RedundantSelfTest.swift new file mode 100644 index 00000000..fbf98b31 --- /dev/null +++ b/src/test/swift/com/sleekbyte/tailor/functional/RedundantSelfTest.swift @@ -0,0 +1,75 @@ +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 anotherDemoMethod(x: Int) { + print(self.x) + 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() + } + } +} + +extension String { + var localized: String { + return NSLocalizedString(self, tableName: nil, bundle: NSBundle.mainBundle(), value: "", comment: "") + } +} + +public override func respondsToSelector(selector: Selector) -> Bool { + switch selector { + case "URLSession:didBecomeInvalidWithError:": + return sessionDidBecomeInvalidWithError != nil + case "URLSession:didReceiveChallenge:completionHandler:": + return sessionDidReceiveChallenge != nil + case "URLSessionDidFinishEventsForBackgroundURLSession:": + return sessionDidFinishEventsForBackgroundURLSession != nil + case "URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:": + return taskWillPerformHTTPRedirection != nil + case "URLSession:dataTask:didReceiveResponse:completionHandler:": + return dataTaskDidReceiveResponse != nil + default: + return self.dynamicType.instancesRespondToSelector(selector) + } +}