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

Is there any plan to add initialisms rule? #283

Open
dengliu opened this issue May 4, 2021 · 6 comments
Open

Is there any plan to add initialisms rule? #283

dengliu opened this issue May 4, 2021 · 6 comments

Comments

@dengliu
Copy link

dengliu commented May 4, 2021

Is there any plan to add initialisms rule?

Words in names that are initialisms or acronyms (e.g. "URL" or "NATO") have a consistent case. For example, "URL" should appear as "URL" or "url" (as in "urlPony", or "URLPony"), never as "Url". As an example: ServeHTTP not ServeHttp. For identifiers with multiple initialized "words", use for example "xmlHTTPRequest" or "XMLHTTPRequest".

This rule also applies to "ID" when it is short for "identifier" (which is pretty much all cases when it's not the "id" as in "ego", "superego"), so write "appID" instead of "appId".

https://github.com/golang/go/wiki/CodeReviewComments#initialisms

@cjoudrey
Copy link
Owner

cjoudrey commented May 4, 2021

👋 @dengliu, I could see this being useful for rules like FieldsAreCamelCased. I'm guessing this is why you're bring this up?

I think we'd want to let people configure their initialisms and then we can use that configuration within the rule when applicable.

Given that there are multiple rules that might need this information (e.g. FieldsAreCamelCased, TypesAreCapitalized, InputObjectValuesAreCamelCased, ...), it might make sense to make this a global configuration instead of forcing people to copy/paste the same initialisms in each rule's options.

Here's an example rule that leverages the global configuration:

export function FieldsHaveDescriptions(configuration, context) {
return {
FieldDefinition(node, key, parent, path, ancestors) {
if (
getDescription(node, {
commentDescriptions: configuration.getCommentDescriptions(),
})
) {
return;

If you'd like to help contribute this feature, I'd be happy to discuss ideas here and 👀 review your implementation.

@dengliu
Copy link
Author

dengliu commented May 4, 2021

thanks for the quick @cjoudrey
I'd love to contribute a bit but I cannot make a hard commitment for this as I am not a JS guy.

I agree to give the flexibility for users to define their own list of initialisms. However, I think as a first step, we can start with some common initialisms defined in lib/config.js , just follow the golint example below:
https://github.com/golang/lint/blob/master/lint.go#L770-L800

WDYT?

@cjoudrey
Copy link
Owner

cjoudrey commented May 4, 2021

However, I think as a first step, we can start with some common initialisms defined in lib/config.js , just follow the golint example below:
https://github.com/golang/lint/blob/master/lint.go#L770-L800

Yeah, that's definitely a second option. I'm not too sure about that approach as this might force people to change their schema and changing the casing of a type / field might be considered a breaking change for some implementations.

e.g. If we decide URL is now a initialisms by default, any schema today that uses Url would get a linter error as soon as they upgrade to the new version of graphql-schema-linter and they'd be forced to rename to URL which might require a lot of changes on their end (and possibly a breaking change to the API).

@dengliu
Copy link
Author

dengliu commented May 4, 2021

agree. Ideally would like to introduce the initialisms config map as the example below.
So if we have that, do we need t update multiple rules to honor this list (e.g. FieldsAreCamelCased, TypesAreCapitalized, InputObjectValuesAreCamelCased, ...)
Or if there is a global rule that is checked before the rest of multiple rules.
(Not sure how the rule checking order is implemented.)

module.exports = {
  rules: ['enum-values-sorted-alphabetically'],
  schemaPaths: ['path/to/my/schema/files/**.graphql'],
  customRulePaths: ['path/to/my/custom/rules/*.js'],
  rulesOptions: {
    'enum-values-sorted-alphabetically': { sortOrder: 'lexicographical' }
  },
  initialisms: {
    'ID':    true,
    'IP':    true,
    'JSON':  true
  }
};

@cjoudrey
Copy link
Owner

cjoudrey commented May 4, 2021

Oh, that's an interesting idea too. 🤔

I was just looking at the rules I mentioned above, and they seem to be pretty loose with the definition of camel case:

# Valid
thisIDIsValid: String

So perhaps we could do this as a new rule that checks initialisms everywhere.

Nitpick about your configuration example, I think we could probably go with an array of strings, unless the boolean value has meaning.

@dengliu
Copy link
Author

dengliu commented May 4, 2021

Oh, the reason to use map instead of list is to have the the developer to have options to know what are the common initialism available to config instead having them to spent too much time to compile this list.
It's easier for them to decide wether a given initialism need to be on or off than think of a list of them.

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

2 participants