Skip to content
This repository has been archived by the owner on Oct 3, 2024. It is now read-only.

Commit

Permalink
add prefer-object-literal rule (#79)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrea-guarino-sonarsource authored Sep 27, 2018
1 parent c7c7885 commit 27e9d7e
Show file tree
Hide file tree
Showing 7 changed files with 308 additions and 0 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
* "switch" statements should have at least 3 "case" clauses ([`no-small-switch`])
* "catch" clauses should do more than rethrow ([`no-useless-catch`])
* Local variables should not be declared and then immediately returned or thrown ([`prefer-immediate-return`]) (:wrench: *fixable*)
* Object literal syntax should be used ([`prefer-object-literal`])
* Return of boolean expressions should not be wrapped into an "if-then-else" statement ([`prefer-single-boolean-return`])
* A "while" loop should be used instead of a "for" loop ([`prefer-while`]) (:wrench: *fixable*)

Expand All @@ -52,6 +53,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
[`no-use-of-empty-return-value`]: ./docs/rules/no-use-of-empty-return-value.md
[`no-useless-catch`]: ./docs/rules/no-useless-catch.md
[`prefer-immediate-return`]: ./docs/rules/prefer-immediate-return.md
[`prefer-object-literal`]: ./docs/rules/prefer-object-literal.md
[`prefer-single-boolean-return`]: ./docs/rules/prefer-single-boolean-return.md
[`prefer-while`]: ./docs/rules/prefer-while.md

Expand Down
27 changes: 27 additions & 0 deletions docs/rules/prefer-object-literal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# prefer-object-literal

Object literal syntax, which initializes an object's properties inside the object declaration is cleaner and clearer than the alternative: creating an empty object, and then giving it properties one by one.

An issue is raised when the following pattern is met:

* An empty object is created.
* A consecutive single-line statement adds a property to the created object.

## Noncompliant Code Example

```javascript
var person = {}; // Noncompliant
person.firstName = "John";
person.middleInitial = "Q";
person.lastName = "Public";
```

## Compliant Solution

```javascript
var person = {
firstName: "John",
middleInitial: "Q",
lastName: "Public",
};
```
28 changes: 28 additions & 0 deletions ruling/snapshots/prefer-object-literal
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
src/angular.js/i18n/src/converter.js: 38
src/angular.js/src/ngAnimate/animateCss.js: 600
src/brackets/src/extensibility/ExtensionManager.js: 313
src/brackets/src/extensions/default/HealthData/HealthDataManager.js: 53
src/brackets/src/extensions/default/InlineColorEditor/ColorEditor.js: 557,570,582
src/brackets/src/extensions/default/JavaScriptRefactoring/RefactoringUtils.js: 365
src/brackets/src/LiveDevelopment/LiveDevServerManager.js: 97
src/Ghost/core/server/helpers/index.js: 1
src/Ghost/core/server/helpers/navigation.js: 64
src/Ghost/core/server/lib/image/image-size.js: 23
src/Ghost/core/server/update-check.js: 87
src/jest/packages/pretty-format/perf/test.js: 182
src/jquery/external/sinon/sinon.js: 4500
src/reveal.js/plugin/search/search.js: 165
src/spectrum/api/models/user.js: 407
src/three.js/editor/js/Command.js: 32
src/three.js/editor/js/History.js: 164
src/three.js/src/core/Geometry.js: 1286,1338
src/three.js/src/core/Object3D.js: 649
src/vue/packages/vue-server-renderer/basic.js: 3943
src/vue/packages/vue-server-renderer/build.js: 3686
src/vue/packages/vue-template-compiler/browser.js: 3058
src/vue/packages/vue-template-compiler/build.js: 2660
src/vue/packages/weex-template-compiler/build.js: 1459
src/vue/packages/weex-vue-framework/factory.js: 3570,3572,5203
src/vue/src/compiler/parser/index.js: 373
src/vue/src/core/global-api/index.js: 22
src/vue/src/core/instance/state.js: 314,316
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const sonarjsRules: [string, Linter.RuleLevel][] = [
["no-use-of-empty-return-value", "error"],
["no-useless-catch", "error"],
["prefer-immediate-return", "error"],
["prefer-object-literal", "error"],
["prefer-single-boolean-return", "error"],
["prefer-while", "error"],
];
Expand Down
96 changes: 96 additions & 0 deletions src/rules/prefer-object-literal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* eslint-plugin-sonarjs
* Copyright (C) 2018 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
// https://jira.sonarsource.com/browse/RSPEC-2428

import { Rule, SourceCode } from "eslint";
import { Node, Statement, Program, Identifier, BlockStatement, Expression } from "estree";
import {
isModuleDeclaration,
isVariableDeclaration,
isObjectExpression,
isExpressionStatement,
isAssignmentExpression,
isMemberExpression,
isIdentifier,
} from "../utils/nodes";
import { areEquivalent } from "../utils/equivalence";

const MESSAGE =
"Declare one or more properties of this object inside of the object literal syntax instead of using separate statements.";

const rule: Rule.RuleModule = {
create(context: Rule.RuleContext) {
return {
BlockStatement: (node: Node) => checkObjectInitialization((node as BlockStatement).body, context),
Program: (node: Node) => {
const statements = (node as Program).body.filter(
(statement): statement is Statement => !isModuleDeclaration(statement),
);
checkObjectInitialization(statements, context);
},
};
},
};

function checkObjectInitialization(statements: Statement[], context: Rule.RuleContext) {
let index = 0;
while (index < statements.length - 1) {
const objectDeclaration = getObjectDeclaration(statements[index]);
if (objectDeclaration && isIdentifier(objectDeclaration.id)) {
if (isPropertyAssignement(statements[index + 1], objectDeclaration.id, context.getSourceCode())) {
context.report({ message: MESSAGE, node: objectDeclaration });
}
}
index++;
}
}

function getObjectDeclaration(statement: Statement) {
if (isVariableDeclaration(statement)) {
return statement.declarations.find(declaration => !!declaration.init && isEmptyObjectExpression(declaration.init));
}
return undefined;
}

function isEmptyObjectExpression(expression: Expression) {
return isObjectExpression(expression) && expression.properties.length === 0;
}

function isPropertyAssignement(statement: Statement, objectIdentifier: Identifier, sourceCode: SourceCode) {
if (isExpressionStatement(statement) && isAssignmentExpression(statement.expression)) {
const { left, right } = statement.expression;
if (isMemberExpression(left)) {
return (
!left.computed &&
isSingleLineExpression(right, sourceCode) &&
areEquivalent(left.object, objectIdentifier, sourceCode)
);
}
}
return false;
}

function isSingleLineExpression(expression: Expression, sourceCode: SourceCode) {
const first = sourceCode.getFirstToken(expression)!.loc;
const last = sourceCode.getLastToken(expression)!.loc;
return first.start.line === last.end.line;
}

export = rule;
15 changes: 15 additions & 0 deletions src/utils/nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
import { Rule } from "eslint";
import * as estree from "estree";

const MODULE_DECLARATION_NODES = [
"ImportDeclaration",
"ExportNamedDeclaration",
"ExportDefaultDeclaration",
"ExportAllDeclaration",
];

export function getParent(context: Rule.RuleContext) {
const ancestors = context.getAncestors();
return ancestors.length > 0 ? ancestors[ancestors.length - 1] : undefined;
Expand Down Expand Up @@ -89,6 +96,14 @@ export function isMemberExpression(node: estree.Node | undefined): node is estre
return node !== undefined && node.type === "MemberExpression";
}

export function isModuleDeclaration(node: estree.Node | undefined): node is estree.ModuleDeclaration {
return node !== undefined && MODULE_DECLARATION_NODES.includes(node.type);
}

export function isObjectExpression(node: estree.Node | undefined): node is estree.ObjectExpression {
return node !== undefined && node.type === "ObjectExpression";
}

export function isReturnStatement(node: estree.Node | undefined): node is estree.ReturnStatement {
return node !== undefined && node.type === "ReturnStatement";
}
Expand Down
139 changes: 139 additions & 0 deletions tests/rules/prefer-object-literal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* eslint-plugin-sonarjs
* Copyright (C) 2018 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import { RuleTester } from "eslint";

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
import rule = require("../../src/rules/prefer-object-literal");

ruleTester.run("prefer-literal", rule, {
valid: [
{
code: `var x = {a: 2}`,
},
{
code: `
function Foo(a) {
this.a = a;
};
var x = new Foo(2);`,
},
{
code: `
var x = {a: 2};
y = "foo";`,
},
// FN
{
code: `
var x;
x = {};
x.a = 2`,
},
// FN
{
code: `var x = {a: 2}; doSomething(); x.b = 3;`,
},
{
code: `
function foo() {
var x = {a: 2};
doSomething();
}`,
},
{
code: `var x = {}; x["a"] = 2;`,
},
// No issue on multiline expressions, may be done for readibility
{
code: `
var x = {};
x.foo = function () {
doSomething();
}
var y = {};
y.prop = {
a: 1,
b: 2
}`,
},
// OK, report only when empty object
{
code: `var x = {a: 2}; x.b = 5;`,
},
],
invalid: [
{
code: `var x = {}; x.a = 2;`,
errors: [
{
message:
"Declare one or more properties of this object inside of the object literal syntax instead of using separate statements.",
line: 1,
endLine: 1,
column: 5,
endColumn: 11,
},
],
},
{
code: `
var x = {},
y = "hello";
x.a = 2;`,
errors: [
{
message:
"Declare one or more properties of this object inside of the object literal syntax instead of using separate statements.",
line: 2,
endLine: 2,
column: 13,
endColumn: 19,
},
],
},
{
code: `var x = {}; x.a = 2; x.b = 3`,
errors: 1,
},
{
code: `let x = {}; x.a = 2;`,
errors: 1,
},
{
code: `const x = {}; x.a = 2;`,
errors: 1,
},
{
code: `{ var x = {}; x.a = 2; }`,
errors: 1,
},
{
code: `if (a) { var x = {}; x.a = 2; }`,
errors: 1,
},
{
code: `function foo() {
var x = {};
x.a = 2;
}`,
errors: 1,
},
],
});

0 comments on commit 27e9d7e

Please sign in to comment.