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

Generic regex rules #123

Closed
keith opened this issue Sep 4, 2015 · 6 comments
Closed

Generic regex rules #123

keith opened this issue Sep 4, 2015 · 6 comments
Labels
enhancement Ideas for improvements of existing features and rules.

Comments

@keith
Copy link
Collaborator

keith commented Sep 4, 2015

As we've been expanding our usage of SwiftLint we've added a lot of simple regex rules to conform to our style guide. I'm not sure what the long term plans are to handle custom rules but it might be nice to provide a way for users to add regex rules in their .swiftlint.yml. For example:

regex:
  - (^[^\\s]+\\s+@objc|@objc[^\\n_])

This would create multiple rules (once #106 is merged) at runtime.

There are a few issues I can see with this, specifically around them not being customizable. I'm not sure how the warning/error message would be generated and things like NSRegularExpression options wouldn't be unusable.

I just wanted to get some feedback on this idea before putting time into it. Let me know your thoughts!

@jpsim
Copy link
Collaborator

jpsim commented Sep 4, 2015

SwiftLint's strength IMO comes from its contextual knowledge of the AST, which is why regex-based rules also typically check the syntax type of its matched tokens. So ideally, the ability to define custom rules in the configuration file should also expose that.

How about providing more information for new rules?

custom_rules:
  ObjCRule:
    message: @objc should be avoided
    regex: (^[^\\s]+\\s+@objc|@objc[^\\n_])
    match_tokens: identifier
    severity: warning

@jpsim
Copy link
Collaborator

jpsim commented Sep 4, 2015

Most of those fields could be optional too.

@keith
Copy link
Collaborator Author

keith commented Sep 4, 2015

SwiftLint's strength IMO comes from its contextual knowledge of the AST

Totally agree. Although I do think for our use case a lot of regex doesn't really matter what section it's in like the one above, and the overhead from interacting with the syntax map isn't always worth it. This obviously doesn't apply to all linting, otherwise we'd all be using Mayday!

@jpsim
Copy link
Collaborator

jpsim commented Sep 4, 2015

Most of the time, you won't care about the AST other than knowing you're not in a comment or string, so it could be worth exposing a helper for that.

@jpsim jpsim added the enhancement Ideas for improvements of existing features and rules. label Sep 19, 2015
@scottrhoyt
Copy link
Contributor

I'm about 75% of the way done with this using the framework from #391. By the time that is merged, I should shortly have a new PR to add this.

@scottrhoyt
Copy link
Contributor

I've got this completed now. Will submit PR after #391. I'm a big fan because it really opens up some cool customization... including pranking your coworkers :)

@scottrhoyt scottrhoyt mentioned this issue Jan 25, 2016
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules.
Projects
None yet
Development

No branches or pull requests

3 participants