Skip to content

Inherit allow_zero_lintable_files from parent config - #6895

Closed
shubhransh-gupta wants to merge 1 commit into
realm:mainfrom
shubhransh-gupta:fix/allow-zero-lintable-files-parent-config
Closed

shubhransh-gupta wants to merge 1 commit into
realm:mainfrom
shubhransh-gupta:fix/allow-zero-lintable-files-parent-config

Conversation

@shubhransh-gupta

Copy link
Copy Markdown

Summary

Resolves #6240.

When merging a parent configuration with a child configuration via parent_config, Configuration+Merging.swift was taking childConfiguration.allowZeroLintableFiles. Because allow_zero_lintable_files defaults to false when unspecified in the child config dictionary, the child config was inadvertently overriding and discarding allow_zero_lintable_files: true defined in the parent config.

Changes

  • In Configuration+Merging.swift, merge allowZeroLintableFiles using childConfiguration.allowZeroLintableFiles || allowZeroLintableFiles.
  • Added unit test allowZeroLintableFilesMerging in Tests/FileSystemAccessTests/ConfigurationTests.swift.
  • Added CHANGELOG.md entry under Bug Fixes following repository guidelines.

@SwiftLintBot

Copy link
Copy Markdown
20 Messages
📖 Building this branch resulted in the same binary size as when built on main.
📖 Linting Aerial with this PR took 0.68 s vs 0.69 s on main (1% faster).
📖 Linting Alamofire with this PR took 0.98 s vs 0.97 s on main (1% slower).
📖 Linting Brave with this PR took 6.35 s vs 6.33 s on main (0% slower).
📖 Linting Brigade with this PR took 18.04 s vs 18.23 s on main (1% faster).
📖 Linting DuckDuckGo with this PR took 29.97 s vs 29.7 s on main (0% slower).
📖 Linting Firefox with this PR took 11.41 s vs 11.45 s on main (0% faster).
📖 Linting Kickstarter with this PR took 7.32 s vs 7.34 s on main (0% faster).
📖 Linting Moya with this PR took 0.38 s vs 0.38 s on main (0% slower).
📖 Linting NetNewsWire with this PR took 2.53 s vs 2.53 s on main (0% slower).
📖 Linting Nimble with this PR took 0.56 s vs 0.56 s on main (0% slower).
📖 Linting PocketCasts with this PR took 7.36 s vs 7.35 s on main (0% slower).
📖 Linting Quick with this PR took 0.36 s vs 0.36 s on main (0% slower).
📖 Linting Realm with this PR took 2.98 s vs 2.97 s on main (0% slower).
📖 Linting Sourcery with this PR took 1.62 s vs 1.65 s on main (1% faster).
📖 Linting Swift with this PR took 4.52 s vs 4.53 s on main (0% faster).
📖 Linting SwiftLintPerformanceTests with this PR took 0.18 s vs 0.18 s on main (0% slower).
📖 Linting VLC with this PR took 1.3 s vs 1.3 s on main (0% slower).
📖 Linting Wire with this PR took 16.26 s vs 16.3 s on main (0% faster).
📖 Linting WordPress with this PR took 10.48 s vs 10.54 s on main (0% faster).

Generated by 🚫 Danger

@SimplyDanny

Copy link
Copy Markdown
Collaborator

Did you check the history of #6240? The issue thread, open PR, comments, ...? The implementation requires more than what you and others proposed and requires careful consideration.

@shubhransh-gupta

Copy link
Copy Markdown
Author

Thanks for pointing that out, @SimplyDanny.

I reviewed the history across #6240, #5724, and the open draft in #6874. You're completely right:

  1. A localized || merge is flawed because it prevents a child configuration from explicitly overriding a parent setting with allow_zero_lintable_files: false.
  2. The same implicit default overwrite issue affects all scalar options (strict, lenient, indentation, baseline, write_baseline, check_for_updates), so handling only allow_zero_lintable_files creates inconsistent merge behavior.
  3. PR Fix parent-child scalar configuration inheritance #6874 already provides the proper explicit-key provenance tracking to address this across all scalar options.

Closing this PR in favor of #6874.

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.

allow_zero_lintable_files: true doesn't work when added to a parent config that's loaded by a child config

3 participants