Add `name-property-casing` rule. #94

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.

Sign up for GitHub

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

Jump to bottom

Merged

michalsnik merged 3 commits into vuejs:master from armano2:patch-14-name-casing

Jul 22, 2017

Contributor

armano2 commented Jul 18, 2017 •

edited

Loading

This PR implements rules proposed in #93

DONE:

Create documtation
Add tests
Implement rule

armano2 mentioned this pull request

Rule Proposition: name property casing #93

Closed


          Add name-casing rule.

armano2 force-pushed the patch-14-name-casing branch from fa9e504 to 8502971 Compare

July 19, 2017 17:51

mysticatea added Ready for review semver-minor labels

armano2 changed the title ~~Add name-casing rule.~~ Add name-property-casing rule.


          rename rule name-casing -> name-property-casing

2f063d1

michalsnik suggested changes

View reviewed changes

docs/rules/name-property-casing.md Outdated

		@@ -0,0 +1,37 @@
		# Name property casing for consistency purposes (name-property-casing)

Member

michalsnik Jul 21, 2017

Requires specific casing for the name property in Vue components (name-property-casing)

docs/rules/name-property-casing.md Outdated

		@@ -0,0 +1,37 @@
		# Name property casing for consistency purposes (name-property-casing)

		Define a style for the `name` property casing for consistency purposes

Member

michalsnik Jul 21, 2017

Missing dot at the end of the sentence

docs/rules/name-property-casing.md Outdated

+              Default casing is set to `PascalCase`
+              ```
+              'vue/name-property-casing': [2, 'camelCase'|'kebab-case'|'PascalCase']

Member

michalsnik Jul 21, 2017

camelCase|kebab-case|PascalCase

lib/rules/name-property-casing.js Outdated

		@@ -0,0 +1,79 @@
		/**
		* @fileoverview Name property casing for consistency purposes

Member

michalsnik Jul 21, 2017

Use the same title as in documentation

lib/rules/name-property-casing.js Outdated

+                return utils.executeOnVueComponent(context, (obj) => {
+                  const node = obj.properties
+                    .filter(item => item.type === 'Property' && item.key.name === 'name' && item.value.type === 'Literal')[0]

Member

michalsnik Jul 21, 2017

Please wrap this line to make it more readable

lib/rules/name-property-casing.js Outdated

+                return utils.executeOnVueComponent(context, (obj) => {
+                  const node = obj.properties
+                    .filter(item => item.type === 'Property' && item.key.name === 'name' && item.value.type === 'Literal')[0]
+                  if (node) {

Member

michalsnik Jul 21, 2017

Add empty line above if

Member

michalsnik Jul 21, 2017

Btw. you can also do:

if (!node) { return; }

To not introduce unnecessary indentation.

lib/rules/name-property-casing.js Outdated

+              }
+              function convertCase (str, caseType) {
+                if (caseType === 'kebab-case') {

Member

michalsnik Jul 21, 2017

Ideal case for switch/case. Or better you can create map with converters:

const convertersMap = {
  'kebab-case': kebabCase,
  'camelCase': camelCase,
  'PascalCase': pascalCase
}

function getConverter(name) {
  return convertersMap[name] || pascalCase
}

And use it like so:

function convertCase (str, caseType) {
  return getConverter(caseType)(str)
}

WDYT?

lib/rules/name-property-casing.js Outdated

+              function create (context) {
+                const options = context.options[0]
+                const caseType = ['camelCase', 'kebab-case', 'PascalCase'].indexOf(options) !== -1 ? options : 'PascalCase'

Member

michalsnik Jul 21, 2017

Please move the array with possible caseTypes out of the create function, it's how it's done in most rules I think.

tests/lib/rules/name-property-casing.js

+                    filename: 'test.js',
+                    code: `
+                      new Vue({
+                        name: 'foo!bar'

Member

michalsnik Jul 21, 2017

Please add one more test with foo_bar


          Apply changes requested in code review.

cd6f6c3

Contributor Author

armano2 commented Jul 21, 2017

@michalsnik i applied your suggestions & i wrapped few additional line of code.

michalsnik approved these changes

View reviewed changes

Member

michalsnik left a comment

Awesome! GJ @armano2 🎉

michalsnik merged commit 29d1cb6 into vuejs:master

michalsnik removed the Ready for review label

Member

michalsnik commented Jul 22, 2017

I'll release it later :)

armano2 deleted the patch-14-name-casing branch

July 22, 2017 12:11

F3n67u commented Dec 8, 2018

Make name-property-casing fixable is very dangerous, because the fix code only fix the name and leave the reference code same. I think the fix should leave to manual fix, we just show the alert and do nothing else.

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

Labels