PureXML/third_party/rules-swift/exp/common-mistakes.md

7.0 KiB

Common Mistakes to Avoid

Anti-patterns paired with the canonical fix. Mini-DOs are inline; full canonical forms live in package-swift.md (closure-with-locals), testing.md (test-target placement), and implementation-patterns.md (Pattern 6 for fonts).

❌ DON'T: Create God Targets

// WRONG: "Shared" target with everything
Packages/Sources/Shared/
├── Models/
├── Views/
├── Networking/
├── Database/
└── Utilities/

✅ DO: Split into focused targets

Packages/Sources/
├── SharedModels/
├── SharedViews/
├── NetworkClient/
├── DatabaseClient/
└── SharedUtilities/

❌ DON'T: Circular Dependencies

// WRONG
AuthFeature depends on AppFeature
AppFeature depends on AuthFeature

✅ DO: Extract shared code

AuthFeature depends on SharedAuthModels
AppFeature depends on SharedAuthModels

❌ DON'T: Inline deps, allProducts, or targets arrays

// WRONG: inline definitions
let deps: [Package.Dependency] = [
    .package(url: "...", from: "1.10.3"),
    .package(url: "...", from: "1.8.3"),
    // ... dozens more
]

let allProducts: [Product] = [
    .library(name: "ApiShared", targets: ["ApiShared"]),
    .library(name: "ApiClient", targets: ["ApiClient"]),
    // ... many more
]

let targets: [Target] = [
    Target.target(name: "SharedModels", dependencies: []),
    Target.testTarget(name: "SharedModelsTests", dependencies: ["SharedModels"]),
    // ... more inline
]

✅ DO: closure-with-local-variables pattern, grouped by purpose / layer / platform. Mini-shape for each array:

// deps
let deps: [Package.Dependency] = {
    // OpenAPI stack
    let openAPIRuntimeDep = Package.Dependency.package(url: "...", from: "1.8.3")
    // Vapor stack
    let vaporDep          = Package.Dependency.package(url: "...", from: "4.119.0")
    return [openAPIRuntimeDep, vaporDep]
}()

// allProducts
let allProducts: [Product] = {
    let apiSharedProduct = Product.singleTargetLibrary("ApiShared")
    let apiClientProduct = Product.singleTargetLibrary("ApiClient")
    return [apiSharedProduct, apiClientProduct]
}()

// targets
let targets: [Target] = {
    // foundation
    let sharedModelsTarget = Target.target(name: "SharedModels", dependencies: [])
    let foundationTargets  = [sharedModelsTarget]
    // api
    let apiClientTarget    = Target.target(name: "ApiClient", dependencies: ["SharedModels"])
    let apiTargets         = [apiClientTarget]
    return foundationTargets + apiTargets
}()

Full forms (with platform splits, helper extensions, every dep / product / target spelled out) in package-swift.md (Dependencies Definition, Product Definition, Target Organization).

❌ DON'T: Skip Test Targets

Every source target MUST ship with a matching Target.testTarget(name: "<name>Tests", dependencies: ["<name>"]), grouped as a pair with the source target in a named sub-array. See testing.md > TEST TARGET PLACEMENT for the canonical form, folder layout, and the rule that mocks live in the test target rather than the source target.

❌ DON'T: Transitive Dependencies

// WRONG: Relying on ApiClient importing SharedModels
import ApiClient
// Using User from SharedModels without importing it

✅ DO: Explicit imports

import ApiClient
import SharedModels  // Explicit dependency

❌ DON'T: Feature-to-Feature Dependencies

// WRONG: Features depending on each other
let appFeatureTarget = Target.target(
    name: "AppFeature",
    dependencies: [
        "AuthFeature",      // ❌ Feature depending on feature
        "ProfileFeature",   // ❌ Creates tight coupling
    ]
)

✅ DO: Coordinator pattern or shared protocols

// Extract navigation/coordination to AppFeature (parent)
// Child features (Auth, Profile) depend on parent's protocols
let authFeatureTarget = Target.target(
    name: "AuthFeature",
    dependencies: [
        "SharedModels",
        "AppCoordination",  // ✅ Protocol target
    ]
)

❌ DON'T: Use Info.plist for Font Registration in SwiftPM Targets

Three anti-patterns, all covered by implementation-patterns.md > Pattern 6 (Font/Resource Target):

  • UIAppFonts / ATSApplicationFontsPath in Info.plist: doesn't work for SwiftPM target resources
  • .copy("Fonts") in Package.swift resources: breaks Bundle.module lookup
  • Bundle.main.urls(...) inside the target: wrong bundle for SwiftPM resources

DO: .process("Fonts") in resources, CTFontManagerRegisterFontsForURL against Bundle.module, called from app init before any UI renders. Full implementation in implementation-patterns.md > Pattern 6.

Acceptance check

Each anti-pattern above is mechanically checkable; the fixes are conforming when:

  • No inline top-level arrays. The three top-level manifest arrays use the closure-with-local-variables form, never a bare array literal:
MANIFEST=Package.swift
[ -f "$MANIFEST" ] || MANIFEST=Packages/Package.swift
grep -nE 'let (deps|allProducts|targets)[^=]*=[[:space:]]*\[' "$MANIFEST"

Output MUST be empty (each should read let deps: [...] = {), per the closure-with-locals fix.

  • No Info.plist font registration in SwiftPM targets. Targets register fonts via CoreText, never the plist keys or .copy("Fonts"):
grep -rnE 'UIAppFonts|ATSApplicationFontsPath|\.copy\("Fonts"\)' Packages/

Output MUST be empty (use .process("Fonts") + CTFontManagerRegisterFontsForURL against Bundle.module, Pattern 6).

  • Every source target has a matching test target. For each Target.target(name: "X") there is a Target.testTarget(name: "XTests"). List the names and confirm no source target lacks its pair:
PACKAGE_ROOT=.
[ -f Package.swift ] || PACKAGE_ROOT=Packages
swift package --package-path "$PACKAGE_ROOT" describe --type json | jq -r '
  .targets as $targets
  | [$targets[] | select(.type == "test") | .name] as $tests
  | $targets[]
  | select(.type == "library" or .type == "executable")
  | . as $source
  | select(($tests | index($source.name + "Tests")) == null)
  | .name
'

Output MUST be empty. Every source target name has a corresponding <name>Tests target.

swift package describe names a library target library; the value regular belongs to dump-package and never occurs in this output, so a filter on it checks no library at all and passes whatever the package holds. To prove the command is not vacuous, count what it selects: the number of library and executable targets MUST equal the number of source targets the manifest declares.

  • Observable checklist for the structural anti-patterns no grep settles, each a clear pass/fail: no "god target" mixing unrelated concerns (Models, Views, Networking, Database under one target); no circular dependency between two targets (the graph is a DAG); no feature target lists another feature target as a dependency (coordination goes through a parent/protocol target); and every cross-target symbol use has an explicit import of the module that declares it, never a reliance on a transitive import.