Skip to content

[Lint] Add a rule to detect and transform [<Type>]() into literal … #617

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

Merged
merged 6 commits into from
Sep 14, 2023
3 changes: 3 additions & 0 deletions Sources/SwiftFormat/Core/Pipelines+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class LintPipeline: SyntaxVisitor {
}

override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(AlwaysUseLiteralForEmptyCollectionInit.visit, for: node)
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
return .visitChildren
}
Expand Down Expand Up @@ -243,6 +244,7 @@ class LintPipeline: SyntaxVisitor {
}

override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(AlwaysUseLiteralForEmptyCollectionInit.visit, for: node)
visitIfEnabled(OmitExplicitReturns.visit, for: node)
visitIfEnabled(UseSingleLinePropertyGetter.visit, for: node)
return .visitChildren
Expand Down Expand Up @@ -356,6 +358,7 @@ extension FormatPipeline {

func rewrite(_ node: Syntax) -> Syntax {
var node = node
node = AlwaysUseLiteralForEmptyCollectionInit(context: context).rewrite(node)
node = DoNotUseSemicolons(context: context).rewrite(node)
node = FileScopedDeclarationPrivacy(context: context).rewrite(node)
node = FullyIndirectEnum(context: context).rewrite(node)
Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftFormat/Core/RuleNameCache+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
@_spi(Testing)
public let ruleNameCache: [ObjectIdentifier: String] = [
ObjectIdentifier(AllPublicDeclarationsHaveDocumentation.self): "AllPublicDeclarationsHaveDocumentation",
ObjectIdentifier(AlwaysUseLiteralForEmptyCollectionInit.self): "AlwaysUseLiteralForEmptyCollectionInit",
ObjectIdentifier(AlwaysUseLowerCamelCase.self): "AlwaysUseLowerCamelCase",
ObjectIdentifier(AmbiguousTrailingClosureOverload.self): "AmbiguousTrailingClosureOverload",
ObjectIdentifier(BeginDocumentationCommentWithOneLineSummary.self): "BeginDocumentationCommentWithOneLineSummary",
Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftFormat/Core/RuleRegistry+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
@_spi(Internal) public enum RuleRegistry {
public static let rules: [String: Bool] = [
"AllPublicDeclarationsHaveDocumentation": false,
"AlwaysUseLiteralForEmptyCollectionInit": false,
"AlwaysUseLowerCamelCase": true,
"AmbiguousTrailingClosureOverload": true,
"BeginDocumentationCommentWithOneLineSummary": false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import Foundation
import SwiftSyntax
import SwiftParser

/// Never use `[<Type>]()` syntax. In call sites that should be replaced with `[]`,
/// for initializations use explicit type combined with empty array literal `let _: [<Type>] = []`
/// Static properties of a type that return that type should not include a reference to their type.
///
/// Lint: Non-literal empty array initialization will yield a lint error.
/// Format: All invalid use sites would be related with empty literal (with or without explicit type annotation).
@_spi(Rules)
public final class AlwaysUseLiteralForEmptyCollectionInit : SyntaxFormatRule {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need Init in the name here? Not that other rule names aren't already a mouthful, but it seems like we could drop it and have the same meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s useful to have it in the name to indicate that this is for initialization only, but I am open to suggestions if you have a better name in mind :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of anything better off the top of my head, so if you think it's helpful to disambiguate, I'm ok with it 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can always change it later if something better comes up :)

public override class var isOptIn: Bool { return true }

public override func visit(_ node: PatternBindingSyntax) -> PatternBindingSyntax {
guard let initializer = node.initializer,
let type = isRewritable(initializer) else {
return node
}

if let type = type.as(ArrayTypeSyntax.self) {
return rewrite(node, type: type)
}

if let type = type.as(DictionaryTypeSyntax.self) {
return rewrite(node, type: type)
}

return node
}

public override func visit(_ param: FunctionParameterSyntax) -> FunctionParameterSyntax {
guard let initializer = param.defaultValue,
let type = isRewritable(initializer) else {
return param
}

if let type = type.as(ArrayTypeSyntax.self) {
return rewrite(param, type: type)
}

if let type = type.as(DictionaryTypeSyntax.self) {
return rewrite(param, type: type)
}

return param
}

/// Check whether the initializer is `[<Type>]()` and, if so, it could be rewritten to use an empty collection literal.
/// Return a type of the collection.
public func isRewritable(_ initializer: InitializerClauseSyntax) -> TypeSyntax? {
guard let initCall = initializer.value.as(FunctionCallExprSyntax.self),
initCall.arguments.isEmpty else {
return nil
}

if let arrayLiteral = initCall.calledExpression.as(ArrayExprSyntax.self) {
return getLiteralType(arrayLiteral)
}

if let dictLiteral = initCall.calledExpression.as(DictionaryExprSyntax.self) {
return getLiteralType(dictLiteral)
}

return nil
}

private func rewrite(_ node: PatternBindingSyntax,
type: ArrayTypeSyntax) -> PatternBindingSyntax {
var replacement = node

diagnose(node, type: type)

if replacement.typeAnnotation == nil {
// Drop trailing trivia after pattern because ':' has to appear connected to it.
replacement.pattern = node.pattern.with(\.trailingTrivia, [])
// Add explicit type annotiation: ': [<Type>]`
replacement.typeAnnotation = .init(type: type.with(\.leadingTrivia, .space)
.with(\.trailingTrivia, .space))
}

let initializer = node.initializer!
let emptyArrayExpr = ArrayExprSyntax(elements: ArrayElementListSyntax.init([]))

// Replace initializer call with empty array literal: `[<Type>]()` -> `[]`
replacement.initializer = initializer.with(\.value, ExprSyntax(emptyArrayExpr))

return replacement
}

private func rewrite(_ node: PatternBindingSyntax,
type: DictionaryTypeSyntax) -> PatternBindingSyntax {
var replacement = node

diagnose(node, type: type)

if replacement.typeAnnotation == nil {
// Drop trailing trivia after pattern because ':' has to appear connected to it.
replacement.pattern = node.pattern.with(\.trailingTrivia, [])
// Add explicit type annotiation: ': [<Type>]`
replacement.typeAnnotation = .init(type: type.with(\.leadingTrivia, .space)
.with(\.trailingTrivia, .space))
}

let initializer = node.initializer!
// Replace initializer call with empty dictionary literal: `[<Type>]()` -> `[]`
replacement.initializer = initializer.with(\.value, ExprSyntax(getEmptyDictionaryLiteral()))

return replacement
}

private func rewrite(_ param: FunctionParameterSyntax,
type: ArrayTypeSyntax) -> FunctionParameterSyntax {
guard let initializer = param.defaultValue else {
return param
}

emitDiagnostic(replace: "\(initializer.value)", with: "[]", on: initializer.value)
return param.with(\.defaultValue, initializer.with(\.value, getEmptyArrayLiteral()))
}

private func rewrite(_ param: FunctionParameterSyntax,
type: DictionaryTypeSyntax) -> FunctionParameterSyntax {
guard let initializer = param.defaultValue else {
return param
}

emitDiagnostic(replace: "\(initializer.value)", with: "[:]", on: initializer.value)
return param.with(\.defaultValue, initializer.with(\.value, getEmptyDictionaryLiteral()))
}

private func diagnose(_ node: PatternBindingSyntax, type: ArrayTypeSyntax) {
var withFixIt = "[]"
if node.typeAnnotation == nil {
withFixIt = ": \(type) = []"
}

let initCall = node.initializer!.value
emitDiagnostic(replace: "\(initCall)", with: withFixIt, on: initCall)
}

private func diagnose(_ node: PatternBindingSyntax, type: DictionaryTypeSyntax) {
var withFixIt = "[:]"
if node.typeAnnotation == nil {
withFixIt = ": \(type) = [:]"
}

let initCall = node.initializer!.value
emitDiagnostic(replace: "\(initCall)", with: withFixIt, on: initCall)
}

private func emitDiagnostic(replace: String, with fixIt: String, on: ExprSyntax?) {
diagnose(.refactorIntoEmptyLiteral(replace: replace, with: fixIt), on: on)
}

private func getLiteralType(_ arrayLiteral: ArrayExprSyntax) -> TypeSyntax? {
guard let elementExpr = arrayLiteral.elements.firstAndOnly,
elementExpr.is(ArrayElementSyntax.self) else {
return nil
}

var parser = Parser(arrayLiteral.description)
let elementType = TypeSyntax.parse(from: &parser)

guard !elementType.hasError, elementType.is(ArrayTypeSyntax.self) else {
return nil
}

return elementType
}

private func getLiteralType(_ dictLiteral: DictionaryExprSyntax) -> TypeSyntax? {
var parser = Parser(dictLiteral.description)
let elementType = TypeSyntax.parse(from: &parser)

guard !elementType.hasError, elementType.is(DictionaryTypeSyntax.self) else {
return nil
}

return elementType
}

private func getEmptyArrayLiteral() -> ExprSyntax {
ExprSyntax(ArrayExprSyntax(elements: ArrayElementListSyntax.init([])))
}

private func getEmptyDictionaryLiteral() -> ExprSyntax {
ExprSyntax(DictionaryExprSyntax(content: .colon(.colonToken())))
}
}

extension Finding.Message {
public static func refactorIntoEmptyLiteral(replace: String, with: String) -> Finding.Message {
"replace '\(replace)' with '\(with)'"
}
}
Loading