nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.9k stars 640 forks source link

SwiftFormat searching parent folders for .swiftformat (how to prevent?) #426

Open iandundas opened 5 years ago

iandundas commented 5 years ago

Hi!

We have a project layout like this:

Project
    .swiftformat # -> *note* this contains `--exclude **/Submodules`
    Source/
        Class.swift
    Submodules/
        SomeFramework/
            .swiftformat # -> here we have another .swiftformat 
            Source/
            Pods/

I'm seeing an issue here where running SwiftFormat from inside the submodule like:

cd Submodules/SomeFramework
Pods/SwiftFormat/CommandLineTool/swiftformat Source  --config .swiftformat --verbose

we get:

Running SwiftFormat...
Skipping /Users/ian/AppsDev/VirtualAffairs/BankingRight/Bankingright-showcase-ios/Submodules
-- ignored
Skipping [path]/Submodules
-- ignored
error: No eligible files found at [path]/Submodules/SomeFramework/Source

So even though we run it from inside the submodule folder, being sure to pass in the .swiftformat found there, it seems SwiftFormat is searching up through several parent folders & finding a .swiftformat file which excludes the /Submodules directory tree, and so ends the execution there.

Is this intended behaviour? I think it's valid that a git submodule could have it's own .swiftformat config without the containing parent getting involved (particularly when swiftformat is itself ran from inside the submodule too).

I've tried re-enabling the Submodules tree from inside /Submodules/SomeFramework/.swiftformat by passing various things to the new --unexclude flag, but I can't get it to work.

All advice gratefully received :)

iandundas commented 5 years ago

The .swiftformat at /Project/.swiftformat is:

# See: https://github.com/nicklockwood/SwiftFormat/blob/master/Rules.md#void

--exclude **/Pods,**/fastlane,**/Submodules

--allman false
--closingparen balanced
--commas inline
--comments indent
--decimalgrouping 3,6
--elseposition next-line
--empty void
--importgrouping testable-bottom
--indent 4
--operatorfunc spaced
--patternlet hoist
--ranges no-space
--self init-only
--semicolons inline
--stripunusedargs always
--trimwhitespace always
--wraparguments before-first
--wrapcollections before-first

whereas the one inside the submodule is:

# See: https://github.com/nicklockwood/SwiftFormat/blob/master/Rules.md#void

--allman false
--closingparen balanced
--commas inline
--comments indent
--decimalgrouping 3,6
--elseposition next-line
--empty void
--importgrouping testable-bottom
--indent 4
--operatorfunc spaced
--patternlet hoist
--ranges no-space
--self init-only
--semicolons inline
--stripunusedargs always
--trimwhitespace always
--wraparguments before-first
--wrapcollections before-first

So basically the same, minus the exclusion

nicklockwood commented 5 years ago

This was deliberate, but this is an undesirable consequence of the design.

The reason it checks parent directories for .swiftformat files is to cope with cases such as the use of git hooks, where the command line tool gets applied to each modified file individually, and wouldn't pick up project-level config files correctly if it only searched the specified file's directory.

nicklockwood commented 5 years ago

A possible solution might be if the user is formatting an entire folder, I ignore inherited exclusion rules that exclude the folder being formatted.

I think that would solve your problem without breaking the git hook case, but there may be edge cases I haven't considered.

nicklockwood commented 5 years ago

@iandundas I completely forgot about the --unexclude feature! You can use --unexclude argument explicitly un-ignore a file that is affected by an inherited exclude rule.

Will that solve your problem?

iandundas commented 5 years ago

Hi Nick, thanks - I do remember seeing the --unexclude option but I think it didn't work for the above, I think it didn't produce a different result. I'm not able to try it again with this at the moment to check conclusively though, unfortunately.

honghaoz commented 5 years ago

I also hit this issue.

I tried with --unexclude, it only works for the full path not for the relative path.

Running the following command under /Submodules/foo works.

swiftformat . --unexclude path/to/Submodules

However, for relative paths like "." or "../", it doesn't work.

In a word, the --unexclude is not helpful for this case.

honghaoz commented 5 years ago

I think one way to resolve this issue is by doing:

  1. Remove --exclude **/Submodules in /Project/.swiftformat
  2. Add a new config file with the following content
    --exclude /Module_doesn't_supoort_swiftformat

    (It has to be case by case based on current implementation)

tarbayev commented 2 years ago

A possible solution might be if the user is formatting an entire folder, I ignore inherited exclusion rules that exclude the folder being formatted.

I think that would solve your problem without breaking the git hook case, but there may be edge cases I haven't considered.

This causes more issues apart from the exclusion rule. In my case I'm getting conflicts for other rules, e.g.: warning: --self option has no effect when redundantSelf rule is disabled

I guess it would make sense to stop searching parent directories once first .swiftformat file is found. This would also remove the need of explicit exclusion of the directories containing local formatting configuration.

nicklockwood commented 2 years ago

I guess it would make sense to stop searching parent directories once first .swiftformat file is found. This would also remove the need of explicit exclusion of the directories containing local formatting configuration.

There's a couple of problems with that:

1) Search may be top down, not bottom up, depending on the starting directory where you run swiftformat from. 2) .swiftformat files are specifically designed to be hierarchical, so you can set common rules at the project root and then selectively enable/disable rules for subdirectories. If we ignored base config when encountering a more local config then you'd have to duplicate all the common rules in each config file.

tarbayev commented 2 years ago

@nicklockwood Thanks for clarification. Would be nice though to have an option for running the formatter also with non-hierarchical configuration. In my case I'm introducing a new module into a legacy project with a different formatting configuration, but I don't want to change anything in the existing project yet. With the hierarchical configuration I have to override all of the non-default rules, which makes the config file more verbose.

bobergj commented 1 year ago

.swiftformat files are specifically designed to be hierarchical, so you can set common rules at the project root and then selectively enable/disable rules for subdirectories. If we ignored base config when encountering a more local config then you'd have to duplicate all the common rules in each config file.

I would say the default should be no config inheritance. Inheritance from a parent folder should be explicitly enabled. The reason being that, if you want to inherit a base config, then you are typically in control of both the parent and child folder hierarchies. On the other hand, when having a git submodule, that submodule may come from a third-party and shouldn't automatically inherit the configuration from your project.

What this could look like in terms of configration:

--extends parent  ## special name that means extend from the nearest parent config
--extends ../path/to/parent/directory/config/file
--extends path/to/company-wide/config/template/in/submodule

The default would be

--extends default  ## extend from the SwiftFormat default rules

It may even be interesting to have a

--extends none  

Which says: don't use any default rules, you have to specify all rules and options explicitly. Which could be useful if you want to be resilient to changes or additions to the SwiftFormat defaults.