Bugs Encountered

bugs.swift.org seems to be down right now. (Well, I get “Service Temporarily Unavailable” anyway.)

So I’m listing these swift-format bugs here instead so that I don’t forget about them and end up with you missing out on the reports altogether. @allevato

They all occurred while using fc5a3da1d5d03143d7a31b12514349e2bf1aba8f, the state of swift-5.1-branch from November 7, 2019. I don’t have the development toolchain to test the bleeding edge, so some or all of these may have already been fixed, but here they are:

  • UseShorthandTypeNames ends up changing some types when it attempts to contract them:

    Optional<(_ nestedInOptional: Bool) -> Void>?
    

    (_ nestedInOptional: Bool) -> Void??
    

    (This is from a deliberately obtuse test fixture; I do not have real code like that.)

  • AlwaysUseLowerCamelCase doesn’t seem to understand caseless characters.

    rename variable 'apache2_0' using lower-camel-case

    rename variable 'gnuGeneralPublic3_0' using lower-camel-case

    Rename variable 'deprecatedPre0_1_1FileName' using lower-camel-case.

  • UseLetInEveryBoundCaseVariable is way out to lunch at times:

    case let simple as SimpleTypeIdentifierSyntax:
    

    distribute 'let' to each bound case variable

    ...?

  • So is CaseIndentLevelEqualsSwitch:

        check(
          for: "\u{2D}",
          allowInFloatLiteral: true,
          allowInToolsVersion: true,
          message: UserFacing<StrictString, InterfaceLocalization>({ localization in
            switch localization {
            // Note to localizers: Adapt the recommendations for the target localization.
            case .englishUnitedKingdom, .englishUnitedStates, .englishCanada:
              return
                "Use a hyphen (‐), minus sign (−), dash (—), bullet (•) or range symbol (–)."
            case .deutschDeutschland:
              return
                "Einen Bindestrich (‐), Minuszeichen (−), Gedankenstrich (–) oder Aufzählungszeichen (•) verwenden."
            }
          }),
        status: status,
        output: output
      )
    

    At switch localization { it objects:

    remove 8 spaces from the indentation of this case

    ...?

  • BlankLineBetweenMembers isn’t consistent with its tranformations:

    private let localizations: [LocalizationIdentifier]
    private let developmentLocalization: LocalizationIdentifier
    private let packageAPI: PackageAPI
    private let api: APIElement
    

    private let localizations: [LocalizationIdentifier]
    
    private let developmentLocalization: LocalizationIdentifier
    private let packageAPI: PackageAPI
    private let api: APIElement
    
  • ValidateDocumentationComments cannot handle named closure parameters:

    /// Creates a lazy option with a resolution algorithm.
    ///
    /// - Parameters:
    ///     - resolve: An closure which resolves the option.
    ///     - configuration: The configuration from which to derive the result.
    public init(resolve: @escaping (_ configuration: WorkspaceConfiguration) -> Option) {
      self.resolve = resolve
    }
    

    replace the plural form of 'Parameters' with a singular inline form of the 'Parameter' tag

    Its message is also hard to understand at times:

    /// Begins creating a symbol page.
    ///
    /// If `coverageCheckOnly` is `true`, initialization will be aborted and `nil` returned once validation is complete. No other circumstances will cause initialization to fail.
    internal convenience init?(
      localization: LocalizationIdentifier,
      allLocalizations: [LocalizationIdentifier],
      // ...
    

    replace the singular inline form of 'Parameter' tag with a plural 'Parameters' tag and group each parameter as a nested list

    Replace what parameter? What’s a tag? Yes, I now the real problem is that the Parameters callout is missing. But the message is confusing because (1) it says to replace something, (2) elsewhere these things are called callouts, not tags and (3) the error points at the function’s parameter list, not the documentation’s, so “replace the [...] Parameter” seems to be objecting to the function signature itself.

In case any of these are only triggered due to something in the wider context, they are all from this repository at revision 91661a3adc3ae3cdddd259f7a0822b5a1ed1fd36.

2 Likes

Thanks for reporting these—it's always great to have more cases to improve our test coverage. It looks like bugs.swift.org is back up, so if you could file these under the swift-format component there, that would be really helpful to individually track them.

We (Google) use swift-format primarily in format mode rather than lint mode ("don't report issues, just fix them and move on"), so the linter is definitely known to be the weaker of the two. We need to improve the testing infrastructure around lint mode; right now we just verify whether or not diagnostics are emitted, but not where, and expressing the assertions is a bit awkward.

To throw in some additional context on some of these:

Hmm, this rule should just be deleted. Indentation solely the purview of the pretty-printer now.

Is there a comment preceding the first of those declarations (possibly with blank lines intervening)? If so, that's a known issue.

We have a lot of improvements planned around our handling of doc comments; the current implementation needs to be replaced using the same Markdown logic used inside the compiler itself. The error's location is currently wrong because we just use the syntax node location, and there's not currently an API to get the location of a specific trivia piece of a syntax node, which I'd like to add to SwiftSyntax.

SR‐11771–11773

I didn’t bother reporting the ones you said will be deleted or completely reimplemented.

1 Like