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

Added new cop - Naming/AsciiConstants #7459

Closed
wants to merge 1 commit into from

Conversation

pjskennedy
Copy link

This adds a new cop: Naming/AsciiConstants

This is an attempt to implement #7458

Rubocop currently has Naming/AsciiIdentifiers which correctly adds offenses to ruby identifiers (tIDENTIFIER) containing non-ascii characters. I find that the same rigor should be applied to constants (tCONSTANT) as the following are currently not offenses:

class Foö
end
module Foö
end
FOÖ = "foo"

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

This PR adds a new cop to add offenses to constants with non-ascii characters
in their declaraction. Currently this exists for identifiers in the
"Naming/AsciiIdentifiers" cop, however this only applies to tIDENTIFIER
and not tCONSTANT.
With this change: class / module / constant declaractions must contain only
ASCII characters to be successful.
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 24, 2019

The proposed cop is fine by me. @rubocop-hq/rubocop-core any objections to including it enabled by default?

@koic
Copy link
Member

koic commented Oct 24, 2019

Both this cop and Naming/AsciiIdentifiers cop conform to The Ruby Style Guide's "English Identifiers" rule. And It seems that most implementations overlap with Naming/AsciiIdentifiers cop.
https://github.com/rubocop-hq/ruby-style-guide#english-identifiers

I think there is a way to enhance the Naming/AsciiIdentifiers cop instead of creating a new cop.
So, for example, it may be better to provide ascii_constants: true (default) option or ascii_constants: false (default) option.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 30, 2019

@koic I like this idea!

@bbatsov bbatsov closed this Mar 21, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 21, 2020

@pjskennedy I'll close this PR, so we can pursue @koic's idea. Thanks for your contribution, though.

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

Successfully merging this pull request may close these issues.

None yet

3 participants