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

add number-literal-case rule - fixes #55 #57

Merged
merged 4 commits into from
Oct 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions docs/rules/number-literal-case.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Enforce lowercase identifier and uppercase value for number literals

Enforces a convention of defining number literals where the literal identifier is written in lowercase and the value in uppercase. Differentiating the casing of the identifier and value clearly separates them and makes your code more readable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include a reasoning here. Maybe:

Differentiating the casing of the identifier and value clearly separates them and makes your code more readable.

Improvement suggestions welcome.


## Fail

```js
const foo = 0XFF;
const foo = 0xff;
const foo = 0Xff;
```

```js
const foo = 0B11;
```

```js
const foo = 0O10;
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the separation between the types might be clearer as different code-blocks than newlines.



## Pass

```js
const foo = 0xFF;
```

```js
const foo = 0b11;
```

```js
const foo = 0o10;
```
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ module.exports = {
'unicorn/filename-case': ['error', {case: 'kebabCase'}],
'unicorn/no-abusive-eslint-disable': 'error',
'unicorn/no-process-exit': 'error',
'unicorn/throw-new-error': 'error'
'unicorn/throw-new-error': 'error',
'unicorn/number-literal-case': 'error'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfmengels Should we do the same as the AVA plugin here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean?

}
}
}
Expand Down
4 changes: 3 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ Configure it in `package.json`.
"unicorn/filename-case": ["error", {"case": "kebabCase"}],
"unicorn/no-abusive-eslint-disable": "error",
"unicorn/no-process-exit": "error",
"unicorn/throw-new-error": "error"
"unicorn/throw-new-error": "error",
"unicorn/number-literal-case": "error"
}
}
}
Expand All @@ -53,6 +54,7 @@ Configure it in `package.json`.
- [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments.
- [no-process-exit](docs/rules/no-process-exit.md) - Disallow `process.exit()`.
- [throw-new-error](docs/rules/throw-new-error.md) - Require `new` when throwing an error. *(fixable)*
- [number-literal-case](docs/rules/number-literal-case.md) - Enforce lowercase identifier and uppercase value for number literals. *(fixable)*


## Recommended config
Expand Down
35 changes: 35 additions & 0 deletions rules/number-literal-case.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';
const fix = value => {
if (!/^0[a-zA-Z]/.test(value)) {
return value;
}

const indicator = value[1].toLowerCase();
const val = value.slice(2).toUpperCase();

return `0${indicator}${val}`;
};

const create = context => {
return {
Literal: node => {
const value = node.raw;
const fixedValue = fix(value);

if (value !== fixedValue) {
context.report({
node,
message: 'Invalid number literal casing',
fix: fixer => fixer.replaceText(node, fixedValue)
});
}
}
};
};

module.exports = {
meta: {
fixable: 'code'
},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really use the meta property. It's not used by ESLint, but the ESLint team uses it for their docs.

@jfmengels ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good to know. I thought that it was used internally or something. I'll drop it.

create
};
75 changes: 75 additions & 0 deletions test/number-literal-case.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/number-literal-case';

const ruleTester = avaRuleTester(test, {
env: {
es6: true
},
parserOptions: {
sourceType: 'module'
}
});

const error = {
ruleId: 'number-literal-case',
message: 'Invalid number literal casing'
};

ruleTester.run('number-literal-case', rule, {
valid: [
'const foo = 0xFF',
'const foo = 0b11',
'const foo = 0o10',
`const foo = '0Xff'`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case does not respect the title of the rule Enforce lowercase identifier and uppercase value for number literals.

Sure, the casing is not the same, but I thought we were enforcing lowercase then uppercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfmengels It's a string that I test here. When I created the rule, I was on my bike thinkering about this "should I explicitely test if the literal is of type number". This is not needed as the raw value of a string is "'0Xff'". With this test, I just wanted to make sure this is not enforced on string values. Does it make sense somehow :p?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad! Didn't look closely enough. That's a good test to have, as you often get surprising results when you're writing tests like that. 👍

],
invalid: [
{
code: 'const foo = 0XFF',
errors: [error],
output: 'const foo = 0xFF'
},
{
code: 'const foo = 0xff',
errors: [error],
output: 'const foo = 0xFF'
},
{
code: 'const foo = 0Xff',
errors: [error],
output: 'const foo = 0xFF'
},
{
code: 'const foo = 0Xff',
errors: [error],
output: 'const foo = 0xFF'
},
{
code: 'const foo = 0B11',
errors: [error],
output: 'const foo = 0b11'
},
{
code: 'const foo = 0O10',
errors: [error],
output: 'const foo = 0o10'
},
{
code: `
const foo = 255;

if (foo === 0xff) {
console.log('invalid');
}
`,
errors: [error],
output: `
const foo = 255;

if (foo === 0xFF) {
console.log('invalid');
}
`
}
]
});