Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore shebang in all rules #1700

Merged
merged 2 commits into from
Jul 26, 2017
Merged

Ignore shebang in all rules #1700

merged 2 commits into from
Jul 26, 2017

Conversation

marcelofabri
Copy link
Collaborator

By making a creative use of regions.
Fixes #1294.

@marcelofabri marcelofabri requested a review from jpsim July 21, 2017 12:59
@SwiftLintBot
Copy link

SwiftLintBot commented Jul 21, 2017

1 Warning
⚠️ Make sure that the docs are updated by running the Generate docs scheme.
12 Messages
📖 Linting Aerial with this PR took 0.37s vs 0.34s on master (8% slower)
📖 Linting Alamofire with this PR took 2.46s vs 2.48s on master (0% faster)
📖 Linting Firefox with this PR took 10.44s vs 10.5s on master (0% faster)
📖 Linting Kickstarter with this PR took 15.68s vs 15.87s on master (1% faster)
📖 Linting Moya with this PR took 0.71s vs 0.73s on master (2% faster)
📖 Linting Nimble with this PR took 1.52s vs 1.48s on master (2% slower)
📖 Linting Quick with this PR took 0.52s vs 0.46s on master (13% slower)
📖 Linting Realm with this PR took 2.22s vs 2.27s on master (2% faster)
📖 Linting SourceKitten with this PR took 0.85s vs 0.81s on master (4% slower)
📖 Linting Sourcery with this PR took 3.67s vs 3.58s on master (2% slower)
📖 Linting Swift with this PR took 10.48s vs 10.32s on master (1% slower)
📖 Linting WordPress with this PR took 10.59s vs 11.2s on master (5% faster)

Generated by 🚫 Danger

@codecov-io
Copy link

codecov-io commented Jul 21, 2017

Codecov Report

Merging #1700 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1700      +/-   ##
==========================================
+ Coverage   87.41%   87.49%   +0.07%     
==========================================
  Files         208      209       +1     
  Lines       10278    10371      +93     
==========================================
+ Hits         8985     9074      +89     
- Misses       1293     1297       +4
Impacted Files Coverage Δ
...SwiftLintFramework/Extensions/File+SwiftLint.swift 93.36% <100%> (+0.19%) ⬆️
Tests/SwiftLintFrameworkTests/RulesTests.swift 100% <100%> (ø) ⬆️
.../SwiftLintFrameworkTests/FileHeaderRuleTests.swift 100% <0%> (ø) ⬆️
.../SwiftLintFrameworkTests/FileLengthRuleTests.swift 100% <0%> (ø)
Tests/SwiftLintFrameworkTests/TestHelpers.swift 72.46% <0%> (+2.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6e7df2...f6a28d9. Read the comment docs.

@marcelofabri marcelofabri force-pushed the mf-ignore-shebang branch 4 times, most recently from 1aab1c5 to f2d105e Compare July 23, 2017 22:50
By making a creative use of regions.
Fixes #1294.
@jpsim
Copy link
Collaborator

jpsim commented Jul 25, 2017

Creative solution! I'm sure there's a "more correct" way, but this should do just fine for now.

How about testing all rules with this?

diff --git a/Tests/SwiftLintFrameworkTests/FileHeaderRuleTests.swift b/Tests/SwiftLintFrameworkTests/FileHeaderRuleTests.swift
index cbe42be6..c75f1389 100644
--- a/Tests/SwiftLintFrameworkTests/FileHeaderRuleTests.swift
+++ b/Tests/SwiftLintFrameworkTests/FileHeaderRuleTests.swift
@@ -32,7 +32,7 @@ class FileHeaderRuleTests: XCTestCase {
             .with(triggeringExamples: triggeringExamples)
 
         verifyRule(description, ruleConfiguration: ["required_string": "**Header"],
-                   stringDoesntViolate: false, skipCommentTests: true, testMultiByteOffsets: false)
+                   stringDoesntViolate: false, skipCommentTests: true, testMultiByteOffsets: false, testShebang: false)
     }
 
     func testFileHeaderWithRequiredPattern() {
diff --git a/Tests/SwiftLintFrameworkTests/FileLenghtRuleTests.swift b/Tests/SwiftLintFrameworkTests/FileLenghtRuleTests.swift
index dcc21f08..f341fa6a 100644
--- a/Tests/SwiftLintFrameworkTests/FileLenghtRuleTests.swift
+++ b/Tests/SwiftLintFrameworkTests/FileLenghtRuleTests.swift
@@ -13,7 +13,7 @@ class FileLenghtRuleTests: XCTestCase {
 
     func testFileLengthWithDefaultConfiguration() {
         verifyRule(FileLengthRule.description, commentDoesntViolate: false,
-                   testMultiByteOffsets: false)
+                   testMultiByteOffsets: false, testShebang: false)
     }
 
     func testFileLengthIgnoringLinesWithOnlyComments() {
diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift
index 9d5169b2..63528cee 100644
--- a/Tests/SwiftLintFrameworkTests/RulesTests.swift
+++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift
@@ -141,7 +141,7 @@ class RulesTests: XCTestCase {
     }
 
     func testLeadingWhitespace() {
-        verifyRule(LeadingWhitespaceRule.description, testMultiByteOffsets: false)
+        verifyRule(LeadingWhitespaceRule.description, testMultiByteOffsets: false, testShebang: false)
     }
 
     func testLegacyCGGeometryFunctions() {
@@ -161,7 +161,7 @@ class RulesTests: XCTestCase {
     }
 
     func testLetVarWhitespace() {
-        verifyRule(LetVarWhitespaceRule.description)
+        verifyRule(LetVarWhitespaceRule.description, testShebang: false)
     }
 
     func testMark() {
@@ -202,7 +202,8 @@ class RulesTests: XCTestCase {
         let description = OperatorUsageWhitespaceRule.description
             .with(nonTriggeringExamples: ["#!/usr/bin/env swift\n"])
             .with(triggeringExamples: []).with(corrections: [:])
-        verifyRule(description, skipCommentTests: true, skipStringTests: true, testMultiByteOffsets: false)
+        verifyRule(description, skipCommentTests: true, skipStringTests: true, testMultiByteOffsets: false,
+                   testShebang: false)
     }
 
     func testPrivateOverFilePrivate() {
diff --git a/Tests/SwiftLintFrameworkTests/TestHelpers.swift b/Tests/SwiftLintFrameworkTests/TestHelpers.swift
index 084b89c0..f132e0fb 100644
--- a/Tests/SwiftLintFrameworkTests/TestHelpers.swift
+++ b/Tests/SwiftLintFrameworkTests/TestHelpers.swift
@@ -135,6 +135,10 @@ private func addEmoji(_ string: String) -> String {
     return "/* 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦 */\n\(string)"
 }
 
+private func addShebang(_ string: String) -> String {
+    return "#!/usr/bin/env swift\n\(string)"
+}
+
 extension XCTestCase {
     // swiftlint:disable:next function_body_length
     func verifyRule(_ ruleDescription: RuleDescription,
@@ -143,7 +147,8 @@ extension XCTestCase {
                     stringDoesntViolate: Bool = true,
                     skipCommentTests: Bool = false,
                     skipStringTests: Bool = false,
-                    testMultiByteOffsets: Bool = true) {
+                    testMultiByteOffsets: Bool = true,
+                    testShebang: Bool = true) {
         guard let config = makeConfig(ruleConfiguration, ruleDescription.identifier) else {
             XCTFail()
             return
@@ -158,6 +163,11 @@ extension XCTestCase {
                            nonTriggers: nonTriggers.map(addEmoji), configuration: config)
         }
 
+        if testShebang {
+            verifyExamples(triggers: triggers.map(addShebang),
+                           nonTriggers: nonTriggers.map(addShebang), configuration: config)
+        }
+
         // Comment doesn't violate
         if !skipCommentTests {
             XCTAssertEqual(

@marcelofabri marcelofabri merged commit 4d61ec1 into master Jul 26, 2017
@marcelofabri marcelofabri deleted the mf-ignore-shebang branch July 26, 2017 08:45
@norio-nomura
Copy link
Collaborator

Thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants