Skip to content

Commit

Permalink
fix: Patch no-unused-expressions rule
Browse files Browse the repository at this point in the history
  • Loading branch information
wcjohnson committed Oct 3, 2018
1 parent 96570de commit db0e309
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/rules/patched.lsc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Patches for core ESLint rules
import noUseBeforeDefine from './patched/noUseBeforeDefine'
import restSpreadSpacing from './patched/rest-spread-spacing'
import noUnusedExpressions from './patched/no-unused-expressions'
import fpNoNil from './patched/fp-no-nil'

patchedRules = {}
Expand Down Expand Up @@ -35,6 +36,7 @@ patches = {
"rest-spread-spacing": patchIfLsc(restSpreadSpacing)
"no-extra-semi": patchIfLsc(nullRule)
"array-callback-return": patchIfLsc(nullRule)
"no-unused-expressions": patchIfLsc(noUnusedExpressions)
"fp/no-nil": patchIfLsc(fpNoNil)
"react/require-render-return": patchIfLsc(nullRule)
}
130 changes: 130 additions & 0 deletions src/rules/patched/no-unused-expressions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/**
* @fileoverview Flag expressions in statement position that do not side effect
* @author Michael Ficarra
*/
"use strict";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: "disallow unused expressions",
category: "Best Practices",
recommended: false,
url: "https://eslint.org/docs/rules/no-unused-expressions"
},

schema: [
{
type: "object",
properties: {
allowShortCircuit: {
type: "boolean"
},
allowTernary: {
type: "boolean"
},
allowTaggedTemplates: {
type: "boolean"
}
},
additionalProperties: false
}
]
},

create(context) {
const config = context.options[0] || {},
allowShortCircuit = config.allowShortCircuit || false,
allowTernary = config.allowTernary || false,
allowTaggedTemplates = config.allowTaggedTemplates || false;

/**
* @param {ASTNode} node - any node
* @returns {boolean} whether the given node structurally represents a directive
*/
function looksLikeDirective(node) {
return node.type === "ExpressionStatement" &&
node.expression.type === "Literal" && typeof node.expression.value === "string";
}

/**
* @param {Function} predicate - ([a] -> Boolean) the function used to make the determination
* @param {a[]} list - the input list
* @returns {a[]} the leading sequence of members in the given list that pass the given predicate
*/
function takeWhile(predicate, list) {
for (let i = 0; i < list.length; ++i) {
if (!predicate(list[i])) {
return list.slice(0, i);
}
}
return list.slice();
}

/**
* @param {ASTNode} node - a Program or BlockStatement node
* @returns {ASTNode[]} the leading sequence of directive nodes in the given node's body
*/
function directives(node) {
return takeWhile(looksLikeDirective, node.body);
}

/**
* @param {ASTNode} node - any node
* @param {ASTNode[]} ancestors - the given node's ancestors
* @returns {boolean} whether the given node is considered a directive in its current position
*/
function isDirective(node, ancestors) {
const parent = ancestors[ancestors.length - 1],
grandparent = ancestors[ancestors.length - 2];

return (parent.type === "Program" || parent.type === "BlockStatement" &&
(/Function/.test(grandparent.type))) &&
directives(parent).indexOf(node) >= 0;
}

/**
* Determines whether or not a given node is a valid expression. Recurses on short circuit eval and ternary nodes if enabled by flags.
* @param {ASTNode} node - any node
* @returns {boolean} whether the given node is a valid expression
*/
function isValidExpression(node) {
if (allowTernary) {

// Recursive check for ternary and logical expressions
if (node.type === "ConditionalExpression") {
return isValidExpression(node.consequent) && isValidExpression(node.alternate);
}
}

if (allowShortCircuit) {
if (node.type === "LogicalExpression") {
return isValidExpression(node.right);
}
}

if (allowTaggedTemplates && node.type === "TaggedTemplateExpression") {
return true;
}

return /^(?:Assignment|Call|New|Update|Yield|Await)Expression$/.test(node.type) ||
(node.type === "UnaryExpression" && ["delete", "void"].indexOf(node.operator) >= 0);
}

return {
ExpressionStatement(node) {
// LSC: if node is implicitly returned, it's fine
if (context.parserServices.isTailNode(node)) return;

if (!isValidExpression(node.expression) && !isDirective(node, context.getAncestors())) {
context.report({ node, message: "Expected an assignment or function call and instead saw an expression." });
}
}
};

}
};
11 changes: 7 additions & 4 deletions src/scope.lsc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ class LscReferencer extends analyze.Referencer {
super(options, scopeManager)
// Tails that will be transformed by the compiler into implicit
// returns or other constructs, indexed by the container node
this.transformedTails = new Map()
this.transformedTails = scopeManager._tailsByNode
this.allTails = scopeManager._tails
this.tailsStack = []
}

Expand All @@ -25,7 +26,9 @@ class LscReferencer extends analyze.Referencer {
////////////////
setTails(node): void -> {
tails = node~getTailExpressions!
if tails.length > 0: this.transformedTails.set(node, tails)
if tails.length > 0:
this.transformedTails.set(node, tails)
for elem t in tails: this.allTails.set(t, true)
this.tailsStack.push(tails)
}

Expand Down Expand Up @@ -259,6 +262,8 @@ class LscScopeManager extends ScopeManager {
this._lscCompilerConfig = compilerConfig
this._lscLodashImports = []
this._lscInlinedOperators = []
this._tailsByNode = new Map()
this._tails = new Map()
}

__nestGlobalScope(node) -> {
Expand Down Expand Up @@ -287,6 +292,4 @@ export analyzeScope(ast, parserOptions, compilerConfig) ->

referencer.visit(ast)

scopeManager._transformedTails = referencer.transformedTails

scopeManager
11 changes: 8 additions & 3 deletions src/services.lsc
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@ import { aliasesFor } from './helpers'
{ isa } = api

export getServices(ast, options, scopeManager, compilerConfig) ->
tailsByNode = scopeManager._tailsByNode
allTails = scopeManager._tails

// Services object
{
// Allow linting rules to inspect the compiler configuration
getCompilerConfig() -> compilerConfig
isLightScript() -> compilerConfig.isLightScript

// Get a list of implicitly returned nodes from the given function
getImplicitlyReturnedNodes(functionNode) ->
rst = scopeManager._transformedTails.get(functionNode)
rst
getImplicitlyReturnedNodes(node) -> tailsByNode.get(node)

// Determine if a node is in tail position and therefore used by
// a LightScript implicit return
isTailNode(node) -> allTails.get(node)

// Babel type aliasing
aliasesFor
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
f() ->
1
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"no-unused-expressions": 1
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]

0 comments on commit db0e309

Please sign in to comment.