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

file_types_order false positive for file only containing extensions #2749

Closed
samrayner opened this issue May 8, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@samrayner
Copy link
Contributor

commented May 8, 2019

New Issue Checklist

Describe the bug

A file containing only extensions is flagged as placed amongst the file type(s) 'main_type'.

Complete output when running SwiftLint, including the stack trace and command used
swiftlint lint --path Test.swift --no-cache --enable-all-rules
Linting Swift files at paths Test.swift
Linting 'Test.swift' (1/1)
/Users/samrayner/Desktop/Test.swift:1:1: warning: File Name Violation: File name should match a type or extension declared in the file (if any). (file_name)
/Users/samrayner/Desktop/Test.swift:1:1: warning: File Types Order Violation: An 'extension' should not be placed amongst the file type(s) 'main_type'. (file_types_order)
Done linting! Found 2 violations, 0 serious in 1 file.
//Test.swift
extension Foo {}
extension Bar {
}

NOTE: This only occurs with the above simplified example if the file name is different to the first extended type. Renaming Test.swift to Foo.swift fixes both warnings.

Environment

  • SwiftLint version (run swiftlint version to be sure)?
    0.32.0
  • Installation method used (Homebrew, CocoaPods, building from source, etc)?
    Homebrew
  • Are you using nested configurations?
    No
  • Which Xcode version are you using (check xcode-select -p)?
    10.2.1

Non-trivial example that triggers the warning twice (first two extensions):

//RequestHeadersStubs.swift
import Foundation
@testable import API

extension UniversalHeaders: Stubbable {
    internal static func stubbed() -> UniversalHeaders {
        return .init(aPIKey: "")
    }
}

extension StandardHeaders: Stubbable {
    internal static func stubbed() -> StandardHeaders {
        return .init(environment: .design)
    }
}

extension AuthorizationHeaders: Stubbable {
    internal static func stubbed() -> AuthorizationHeaders {
        return .init(environment: .design, authorization: "")
    }
}
@Dschee

This comment has been minimized.

Copy link
Collaborator

commented May 9, 2019

@samrayner Actually this was the intended behavior as file_type_order was designed with expecting "file_name" being turned on as well. But that is not documented and is probably not a good idea given your code example anyways. So making an exception for cases when the file only contains extensions might be a good idea.

I think the easiest solution to this is to add an early return in this line when none of the fileType attributes in the orderedFileTypeOffsets dictionaries values is mainType. Then we could skip the file_type_order rule entirely if configuration.order contains mainType. (If it doesn't, we should not early-return.)

@samrayner Would you be up to implement this yourself and post a PR? I've tried to lay out how this could be tackled. Just make sure to write a failing-test with your above example first before fixing it so we can ensure this doesn't reappear sometime in the future. Also don't forget to update the Changelog. :)

samrayner added a commit to samrayner/SwiftLint that referenced this issue May 9, 2019

realm#2749 Fix file_types_order flagging extensions
An attempt to prevent files only containing extensions being flagges by the file_types_order rule

samrayner added a commit to samrayner/SwiftLint that referenced this issue May 10, 2019

realm#2749 Fix file_types_order flagging extensions
Prevents extensions being flagged as in the wrong order when a file only contains extensions and nothing else (enums, classes, structs). In this case it's not intuitive that the longest extension be considered the "main" one so the file can be considered as having no "main" type.

samrayner added a commit to samrayner/SwiftLint that referenced this issue May 10, 2019

realm#2749 Fix file_types_order in extension-only files
Prevents extensions being flagged as in the wrong order when a file only contains extensions and nothing else (enums, classes, structs). In this case it's not intuitive that the longest extension be considered the "main" one so the file can be considered as having no "main" type.
@samrayner

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2019

Thanks @Dschee! Pull request here: #2754

samrayner added a commit to samrayner/SwiftLint that referenced this issue May 10, 2019

realm#2749 Fix file_types_order in extension-only files
Prevents extensions being flagged as in the wrong order when a file only contains extensions and nothing else (enums, classes, structs). In this case it's not intuitive that the longest extension be considered the "main" one so the file can be considered as having no "main" type.

samrayner added a commit to samrayner/SwiftLint that referenced this issue May 13, 2019

realm#2749 Fix file_types_order in extension-only files
Prevents extensions being flagged as in the wrong order when a file only contains extensions and nothing else (enums, classes, structs). In this case it's not intuitive that the longest extension be considered the "main" one so the file can be considered as having no "main" type.

Dschee added a commit that referenced this issue May 14, 2019

Merge pull request #2754 from samrayner/samrayner/file-types-order-ex…
…tensions

#2749 Fix file_types_order in extension-only files

@samrayner samrayner closed this May 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.