Skip to content

Feat: Adding a script to run SwiftLint locally#2095

Open
AntAmazonian wants to merge 16 commits intomainfrom
af/add-lint-script
Open

Feat: Adding a script to run SwiftLint locally#2095
AntAmazonian wants to merge 16 commits intomainfrom
af/add-lint-script

Conversation

@AntAmazonian
Copy link
Copy Markdown

Issue #

N/A

Description of changes

The Swiftlint analyzer is configured to run on aws-sdk-swift CI. The analyzer provides better feedback than the main linter, but it also requires setup, including building the project prior to analysis.

A script is provided that developers can run on desktop to perform the same Swiftlint analysis that CI performs.

New/existing dependencies impact assessment, if applicable

N/A

Conventional Commits

"Add SwiftLint config and local execution script"

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Copy Markdown
Contributor

@sichanyoo sichanyoo left a comment

Choose a reason for hiding this comment

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

Script in this PR only runs lint, not analyzer.

scripts/lint.sh Outdated
SCRIPT_DIR="$(dirname "$0")"
CONFIG_PATH="$SCRIPT_DIR/../.swiftlint.yml"

swiftlint lint --config "$CONFIG_PATH" --strict
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This just runs lint, not analyzer.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Script now builds logger, runs analyzer, and lint

Copy link
Copy Markdown
Contributor

@sichanyoo sichanyoo left a comment

Choose a reason for hiding this comment

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

Is there a particular reason SPM build log was used instead of Xcode build log like in CI? I think the main point of having a script for this is so that we can easily run the analyzer lint the same way it is done in CI, but this technically diverges from it.

Have you run this lint yourself? Not sure if it would take forever to run because it tries to build every service in the project. See

- name: Eliminate AWS service clients
run: |
cd aws-sdk-swift
rm -rf Sources/Services/*
cd AWSSDKSwiftCLI
swift run AWSSDKSwiftCLI generate-package-manifest ..
cd ..
echo
cat Package.swift
- name: Compile project with Xcode
run: |
cd aws-sdk-swift
xcodebuild -scheme aws-sdk-swift-Package -destination platform=macOS > xcodebuild.log
- name: Run swiftlint analyzer
run: |
cd aws-sdk-swift
swiftlint analyze --strict --reporter github-actions-logging --compiler-log-path xcodebuild.log
for how generated SDK is removed before running analyzer.

@AntAmazonian
Copy link
Copy Markdown
Author

Is there a particular reason SPM build log was used instead of Xcode build log like in CI? I think the main point of having a script for this is so that we can easily run the analyzer lint the same way it is done in CI, but this technically diverges from it.

Have you run this lint yourself? Not sure if it would take forever to run because it tries to build every service in the project. See

- name: Eliminate AWS service clients
run: |
cd aws-sdk-swift
rm -rf Sources/Services/*
cd AWSSDKSwiftCLI
swift run AWSSDKSwiftCLI generate-package-manifest ..
cd ..
echo
cat Package.swift
- name: Compile project with Xcode
run: |
cd aws-sdk-swift
xcodebuild -scheme aws-sdk-swift-Package -destination platform=macOS > xcodebuild.log
- name: Run swiftlint analyzer
run: |
cd aws-sdk-swift
swiftlint analyze --strict --reporter github-actions-logging --compiler-log-path xcodebuild.log

for how generated SDK is removed before running analyzer.

My intention is to allow this to be run even without xcode, I have ran in locally from the terminal.

scripts/lint.sh Outdated
# aws-sdk-swift
#
# Created by Felix, Anthony on 1/23/26.
#
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Replace the header with the standard Amazon header. Example:
https://github.com/awslabs/aws-sdk-swift/blob/main/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/AWSClientConfigDefaultsProvider.swift#L1-L6

Alternatively, you can simply delete the header entirely.

Copy link
Copy Markdown
Contributor

@jbelkins jbelkins left a comment

Choose a reason for hiding this comment

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

See individual comments

scripts/lint.sh Outdated
echo "✅ Build completed successfully"

echo "Step 4: Running SwiftLint Analyze (this may take a few minutes)..."
timeout 300 swiftlint analyze --compiler-log-path "$LOG_FILE" || echo "⚠️ SwiftLint Analyze timed out after 5 minutes"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the reason for the 5 minute timeout? I generally disfavor setting timeouts unless they're there to address an operational problem, which I don't think we have here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The timeout I added as a just in case for local runs by the developer, but thinking twice about it now I think this should be more on the developer to set if they want to.

scripts/lint.sh Outdated
export PATH="$PATH:/opt/homebrew/bin:/usr/local/bin"

# Unset AWS_SWIFT_SDK_USE_LOCAL_DEPS to use remote dependencies instead of local ones
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we want to do this for a locally-run development script, since SDK developers are typically using a local copy of smithy-swift with local SDK. If you've made local SDK changes that have corresponding smithy-swift changes, the xcodebuild initiated below will fail.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okay, I didn't think about developers making their own changes if they used a copy of the SDK locally, I'll change it so leaves the deps alone if they were set by the developer. That way it's a choice on their end.

- multiple_closures_with_trailing_closure
- cyclomatic_complexity
- file_length
- syntactic_sugar
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why the specific changes to linter rules in this file?

Also, for simplicity, we typically keep the same linter rules in SDK and in smithy-swift. So if we do change rules here, there should be a 2nd PR to change them there as well.

@AntAmazonian AntAmazonian requested a review from a team as a code owner April 1, 2026 16:12
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.

3 participants