Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwiftLint and danger-swiftlint parse Environment Variable Syntax in .swiftlint.yml differently #105

Open
heidiproske opened this issue Jul 20, 2018 · 3 comments

Comments

@heidiproske
Copy link

heidiproske commented Jul 20, 2018

Issue

Quick Summary

Environment Variable Syntax in .swiftlint.yml is parsed differently by danger-ruby-swiftlint to how it's parsed by swiftlint and results in excluded files defined before the variable to still be linted.

Background

hey @AvdLee / @ashfurrow Moooonths ago I wrote a comment on the thread of PR #87 about how after upgrading to danger-ruby-swiftlint v0.16.0 my excluded files started getting linted (whereas prior to v0.16.0 things worked fine and dandy).

I finally figured out the issue 😮

Current Version Information

Using the danger-swiftlint 0.17.1 and the SwiftLint 0.26.0 binary that ships with it :)

Snippet from my SwiftLint configuration

I'm using a custom variable in my .swiftlint.yml to allow devs to add an additional source folder/file to be ignored by the linter. Notice the presence of ${FL_LINT_CUSTOMIGNORE} inside the excluded section below.

excluded: # paths to ignore during linting. Takes precedence over `included`.
- Pods
# If you wish to exclude an additional file or folder, then define the path in your fastlane/.env under the environmental variable FL_LINT_CUSTOMIGNORE.
- ${FL_LINT_CUSTOMIGNORE}

Testing

If I have this environment variable:

  • set (export FL_LINT_CUSTOMIGNORE="SourceFolder/SomeSubFolder")
  • or not set (export FL_LINT_CUSTOMIGNORE="") (Also tried a string with a single space)

then using this excluded section shown above directly with swiftlint works as desired. (i.e. SwiftLint always excludes the Pods/ folder and correctly ignores the "custom path" too.

However, if I use the same .swiftlint.yml with danger-swiftlint then for projects where this environment variable is set to an empty string, danger seems to pretend there is no excluded section. (I.e. it doesn't even exclude my Pods/ folder :(

Attempted Solutions

If I change the syntax in my excluded section to remove the curly braces i.e. from:
- ${FL_LINT_CUSTOMIGNORE}
to
- $FL_LINT_CUSTOMIGNORE
Then danger-swiftlint does what I want and correctly excludes both Pods/ and my custom path. BUT then SwiftLintignores my custom path :( So this is not an option.

I also tried to see what happens when I have (notice how the second dash has nothing next to it)

excluded: # paths to ignore during linting. Takes precedence over `included`.
- Pods
- 

and danger-swiftlint correctly parses this and knows to exclude my Pods/ folder.

What do I want?

I want danger-swiftlint and SwiftLint to parse environment variables in the .swiftlint.yml in the same manner. But it seems they do the opposite thing.

I know @AvdLee mentioned that the only difference withdanger-swiftlint 0.16.0 to previous versions was the added support for environment variables.

Am I just being silly? Or is there an easier way for me to do what I'd like? (I have 40+ repos all using the same .swiftlint.yml file and some of them need the custom path which I let them define in their fastlane/.env and my linter script does the rest (used from both inside an Xcode build phase and via danger-swiftlint during PR checks).

🙏 🆘

@AvdLee
Copy link
Collaborator

AvdLee commented Jul 20, 2018

Thanks @heidiproske, great found!!

I don't have time soon to fix this, but you can help to make it easier to solve this. Do you know how SwiftLint parses environment variables?

If we can compare that to our implementation we might catch the issue.

@heidiproske
Copy link
Author

@AvdLee okey dokey, I'll investigate some more this weekend, thanks for the pointer! :)

@ashfurrow
Copy link
Owner

@heidiproske Thank you for the detailed issue! To be honest, I had no idea that SwiftLint was parsing out environment variables from the YAML file at all. That makes sense now, but wasn't obvious to me.

I took a quick look at the code that @AvdLee linked to above, and it appears to work. Perplexing. Maybe the YAML load is getting messed up?

As a workaround, the projects that don't need it could define their own nonsense environment variable. What I mean is, if the YAML file gets parsed and SwiftLint is told to ignore some directory named XYZ, and that directory doesn't exist, I would expect SwiftLint to be okay with it. Not sure.

Debugging Ruby can be quite different from debugging iOS apps. I recommend using a debugger called Prythis tutorial gives a really good overview of how to drop into the debugger in certain lines of code. You can even get into the debugger inside a unit test, which makes debugging scenarios that involve a lot of set up easier.

Good luck, and thanks again for all your digging! If I can help, let me know.

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

No branches or pull requests

3 participants