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

Feature Request: Customize initialisms or remove "ID" from the common list #41

Closed
cvbarros opened this issue Jul 17, 2018 · 6 comments
Closed
Assignees

Comments

@cvbarros
Copy link

Is your feature request related to a problem? Please describe.
In short, I'm looking for alternatives to golint, particularly because of this issue:
golang/lint#89

Describe the solution you'd like
There are some options to implement this feature.

  1. We could provide a "whilelist" of allowed initialisms, such as Id, in order to name variables such as projectId via configuration. This is the most flexible option, but may lead to other initialisms than Id to be whitelisted, spreading inconsistency across Go projects.
  2. Remove ID completely from the initialism list. The comment on source code, similarly to Golint, states that "Freudian code is rare". However this rule creates warnings for several projects, such as here
  3. Refactor initialisms into its own rule other than var-naming, so it can be enabled/disabled as whole.

Describe alternatives you've considered
I've tried disabling the var-naming rule, but other effective idiomatic rules get disabled as well, so that's not an option.

Additional context
The lack of flexibility in Golint rules is something that this project can offer, along the speed.

I'd more than happy to give it a try at this myself, if applicable.

@mgechev
Copy link
Owner

mgechev commented Jul 17, 2018

We can make the rule configurable. I'd prefer to keep this check under var-naming so we can be as compatible with golint as possible.

If you have some spare time, it'll be great if you can give it a try! We can discuss the approach you're planning here so we can collect some more thoughts! // cc @chavacava

@chavacava
Copy link
Collaborator

As described by @cvbarros, the configuration with an optional (empty by default) white list of "relaxed" initialisms seems the most flexible approach. It may lead to inconsistencies across projects but not more than adding a new rule (option 3).

@mgechev
Copy link
Owner

mgechev commented Jul 18, 2018

I'm thinking of two options:

  • Allow developers to provide a custom list of initialisms. By default, we use the built-in one
  • Allow developers to add extra valid Initialisms

The second aligns with what you mentioned, and the configuration will be less verbose in case the only initialisms a developer wants to alter is Id, so we can go with it.

@cvbarros
Copy link
Author

I think that 2 will also provide a less verbose, while still flexible approach. Feel free to work on it if you want, otherwise I'll give it a try next week 👍

@mgechev
Copy link
Owner

mgechev commented Jul 18, 2018

@cvbarros I won't be able to work on it shortly. Assigned the issue to you so you can take care.

mgechev added a commit that referenced this issue Sep 15, 2018
This PR introduces a white & black lists of initialisms for the
`var-naming` rule.

Now the rule can be configured with:

```toml
[rule.var-naming]
  arguments = [["ID"], ["VM", "BAR"]]
```

This way, the linter will ignore `customId` but will throw on `customVm` or `customBar`.

Fix #41
mgechev added a commit that referenced this issue Sep 15, 2018
This PR introduces a white & black lists of initialisms for the
`var-naming` rule.

Now the rule can be configured with:

```toml
[rule.var-naming]
  arguments = [["ID"], ["VM", "BAR"]]
```

This way, the linter will ignore `customId` but will throw on `customVm` or `customBar`.

Fix #41
@mgechev
Copy link
Owner

mgechev commented Sep 15, 2018

I implemented this feature right here.

Let me know if you have other suggestions.

mgechev added a commit that referenced this issue Sep 15, 2018
This PR introduces a white & black lists of initialisms for the
`var-naming` rule.

Now the rule can be configured with:

```toml
[rule.var-naming]
  arguments = [["ID"], ["VM", "BAR"]]
```

This way, the linter will ignore `customId` but will throw on `customVm` or `customBar`.

Fix #41
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