Skip to content

style(editor): clear the vendored editor packages' lint violations - #2885

Merged
datlechin merged 1 commit into
mainfrom
chore/editor-style-pass
Sep 15, 2026
Merged

datlechin merged 1 commit into
mainfrom
chore/editor-style-pass

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

The two vendored editor packages carried 502 SwiftLint violations, because .swiftlint.yml has never covered LocalPackages/. They are about to move under Packages/, where #2882 already put the gate, so clearing them first keeps that move a pure move rather than a move plus 502 unrelated edits.

Eighth step of the editor adoption work, after #2873, #2875, #2877, #2878, #2880, #2881, #2882 and #2883. The config itself does not change here; these files come under the gate when they move.

swiftlint --fix is not safe at this scale

I ran it once for the mechanical 353 and had to repair its output four times. Recording it here because the failures are not the kind a reviewer expects:

  • It rewrote public var isEmpty: Bool { count == 0 } into public var isEmpty: Bool { isEmpty } in TextLineStorage. That compiles, and recurses until the stack runs out. Nothing but a test that calls isEmpty would ever catch it.
  • It applied the same empty_count rewrite to three types that have a count and are not Collections: RangeStore, an optional (x?.count == 0 became x?.isEmpty, which is Bool? and does not satisfy #expect), and, earlier in chore(lint): lint TablePro's own SwiftPM packages #2882, a tuple (query: String, count: Int).
  • lower_acl_than_parent dropped public from an OptionSet nested in an extension NSBezierPath, after which init(rawValue: Corners.RawValue) resolved Corners to AppKit's own nested type and stopped compiling. Naming the raw type (Int) fixes it.
  • It also edits files outside the change's scope, because --fix applies correctable rules that lint --strict does not report. Seven app files were reverted.

The remaining 149 were done by hand. The trap that hides all of this: xcodebuild on the app scheme does not compile every package target, so the app build stays green while a package is broken. Both package suites are the gate that matters.

Two things this surfaced that are worth reading

A test had been disabled with an underscore and nobody noticed. identifier_name fired on _test_updateOnResizeNoScrollView in VisibleRangeProviderTests. The leading underscore is how you stop XCTest collecting a method, so it had not run since it was written, and it asserted nothing: it captured visibleSet, resized, and compared. It is now test_noScrollViewMakesTheWholeDocumentVisible, it asserts the contract the name claims, and it runs. That is the XCTest count going from 111 to 112.

TextView._undoManager is now editorUndoManager. identifier_name fired on the leading underscore. It is public and the app's tests read it, so the rename and its one caller in EditorLifecycleTeardownTests are in this commit, per the atomic-API rule.

Directives

Four, each because the rule is wrong for that line:

  • TextLineStorage.isEmpty, empty_count. The rule wants count == 0 written as isEmpty, on the line that defines isEmpty. That is the infinite recursion above, and the directive is what stops the next person's --fix from reintroducing it.
  • Two loadView() overrides, prohibited_super_call. NSViewController.loadView() documents that an override which does not use a nib must set self.view, and these build their view hierarchy on top of the superclass's. Removing the super call empties the view.
  • One storage_environment_defaults. A package cannot import the app target, so it cannot reach AppStorageEnvironment; the value is a defaulted parameter the host overrides.

Verification

  • swiftlint lint --strict over both packages: 502 to 0. Run through a config that adds LocalPackages to included:, since the root config does not cover them yet; that file is not committed.
  • swift test --package-path LocalPackages/CodeEditTextView --force-resolved-versions: 333 of 333.
  • swift test --package-path LocalPackages/CodeEditSourceEditor --force-resolved-versions: 112 XCTest executed, 2 skipped, 0 failures; Swift Testing 102 of 102.
  • App build passes. Project regenerated.

@datlechin
datlechin merged commit 29d016b into main Sep 15, 2026
8 of 11 checks passed
@datlechin
datlechin deleted the chore/editor-style-pass branch September 15, 2026 08:05
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.

1 participant