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

Feature request: option to force exclusion #2051

Closed
ashfurrow opened this Issue Feb 14, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@ashfurrow
Contributor

ashfurrow commented Feb 14, 2018

New Issue Checklist

Feature Request

Hello! I like SwiftLint a lot, so much so that I built a tool called danger-ruby-swiftlint to run SwiftLint on CI with Danger. The way the tool works is it runs SwiftLint against each file that's been changed in a pull request, and reports any violations to GitHub in a comment. However, we've run into a problem, reported here.

Here's the scenario:

  • We run swiftlint individually against each changed file.
  • This includes all files, even those that are excluded in .swiftlint.yml.
  • Those excluded files are still having their violations reported back to GitHub.

To be clear: I don't think this is a bug in SwiftLint. It's just a sort of edge case that I've hit. I had a very similar problem with my very similar danger-rubocop tool, but was able to get around it using Rubocop's --force-exclusion flag, detailed here. This flag, off by default, forces Rubocop to adhere to the exclusions in its config file, even if Rubocop is explicitly invoked with that file.

Could we add something equivalent to Rubocop's --force-exclusions flag to SwiftLint?

I totally understand that this probably isn't a high priority. What I'm really looking for here is a 👍 or 👎 on the feature request itself. If you're amenable to it, just provide a high-level description of how you'd like users to specify they want to force exclusions, and I'll send you a pull request.

Thanks for considering it!

@marcelofabri

This comment has been minimized.

Collaborator

marcelofabri commented Feb 15, 2018

My initial thought on this is that this would be useful if it's not too hard to implement/maintain.

I think having the exact same flag on the swiftlint lint command would work. Would that also be an option when autocorrecting?

Also: do you know about other use cases for this flag? I'm wondering if there's any use case where it'd be helpful to specify it on the configuration file instead of in the command.

@ashfurrow

This comment has been minimized.

Contributor

ashfurrow commented Feb 15, 2018

@marcelofabri that's a good question about use cases. I did some research and the only use cases I can find for Rubocop's --force-exclusion flag are tools built on top of Rubocop.

Concerning whether or not this should be a flag vs a config file option, I don't see any downsides either way. Because SwiftLint doesn't appear to have any flags in the --option style, putting it in the config file would probably be the most practical. If that's okay with you, I'm happy to send a PR and we can discuss the specifics of the implementation further.

I think having the same flag for autocorrecting would make sense.

@marcelofabri

This comment has been minimized.

Collaborator

marcelofabri commented Feb 15, 2018

If the majority of use cases would for tools to use it, I think it'd be better to make it an option instead of a configuration, because then consumers wouldn't need to change anything on their project.

SwiftLint does have some options, so I think it'd be fine:

$ swiftlint help lint
Print lint warnings and errors (default command)

[--path (string)]
	the path to the file or directory to lint

[--use-stdin]
	lint standard input

[--config (string)]
	the path to SwiftLint's configuration file

[--strict]
	fail on warnings

[--lenient]
	downgrades serious violations to warnings, warning threshold is disabled

[--use-script-input-files]
	read SCRIPT_INPUT_FILE* environment variables as files

[--benchmark]
	save benchmarks to benchmark_files.txt and benchmark_rules.txt

[--reporter (string)]
	the reporter used to log errors and warnings

[--quiet]
	don't print status logs like 'Linting <file>' & 'Done linting'

[--cache-path (string)]
	the directory of the cache used when linting

[--no-cache]
	ignore cache when linting

[--enable-all-rules]
	run all rules, even opt-in and disabled ones, ignoring `whitelist_rules`

A PR would be very welcome 💯

@ashfurrow

This comment has been minimized.

Contributor

ashfurrow commented Feb 15, 2018

Cool, thanks for clarifying. I'll send a PR this weekend 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment