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

fix: Support of new select API for all rules #68

Merged
merged 4 commits into from May 4, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 25 additions & 46 deletions src/rules/checkUnusedFluxDependenciesRule.ts
Expand Up @@ -3,16 +3,13 @@
* All rights reserved.
*/

import * as ts from "typescript";
import * as Lint from "tslint";
import { IOptions } from "tslint";
import * as ts from 'typescript';
import * as Lint from 'tslint';
import { IOptions } from 'tslint';
import { getSelectMetadata } from '../utils/selectUtils';
import { getConnectMetadata } from '../utils/connectUtils';

type TFluxDependency = "store" | "selector";

enum FLUX_FACTORY {
CONNECT = "connect",
SELECT = "select"
}
type TFluxDependency = 'store' | 'selector';

const watchedIdentifiers: { [key: string]: TFluxDependency } = {};

Expand All @@ -26,7 +23,7 @@ export class Rule extends Lint.Rules.AbstractRule {

public apply(sourceFile: ts.SourceFile): Array<Lint.RuleFailure> {
return this.applyWithWalker(
new NoUnusedDependenciesWalker(sourceFile, this.getOptions())
new NoUnusedDependenciesWalker(sourceFile, this.getOptions()),
);
}
}
Expand All @@ -45,9 +42,7 @@ const getImports = (node: ts.ImportDeclaration) => {

return [
...collectedIdentifiers,
...(namedImports
? namedImports.elements.map(element => element.name.text)
: [])
...(namedImports ? namedImports.elements.map(element => element.name.text) : []),
];
};

Expand All @@ -58,7 +53,7 @@ const addToWatchedIdentifiers = (name: string, type: TFluxDependency) => {
const addToWatchedIdentifiersFromImportNode = (
node: ts.ImportDeclaration,
type: TFluxDependency,
onlyDefault?: boolean
onlyDefault?: boolean,
) => {
if (onlyDefault) {
const importClause = node.importClause;
Expand Down Expand Up @@ -88,8 +83,8 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
super(sourceFile, options);

const configuration = {
reference: "",
...(options.ruleArguments && options.ruleArguments[0])
reference: '',
...(options.ruleArguments && options.ruleArguments[0]),
} as Configuration;

this.configuration = configuration;
Expand All @@ -101,10 +96,8 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
node.getStart(),
node.getWidth(),
text +
(this.configuration.reference
? ` ${this.configuration.reference}`
: "")
)
(this.configuration.reference ? ` ${this.configuration.reference}` : ''),
),
);
}

Expand All @@ -116,12 +109,10 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
ts.isArrayLiteralExpression(dependencyNode)
) {
const selectorsNodesIdentifiers = dependencyNode.elements.filter(
ts.isIdentifier
ts.isIdentifier,
);

const dependencies = selectorsNodesIdentifiers.map(
element => element.text
);
const dependencies = selectorsNodesIdentifiers.map(element => element.text);

if (
ts.isVariableDeclaration(node.parent) &&
Expand All @@ -137,10 +128,7 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {

usedDependencies.push(identifier);

if (
watchedIdentifiers[identifier] &&
!dependencies.includes(identifier)
) {
if (watchedIdentifiers[identifier] && !dependencies.includes(identifier)) {
this.addIssue(node, Rule.NOT_LISTENING(identifier));
}
};
Expand Down Expand Up @@ -184,7 +172,7 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
ts.forEachChild(implementationNode, cb);

const unusedDependencies = dependencies.filter(
dependency => !usedDependencies.includes(dependency)
dependency => !usedDependencies.includes(dependency),
);

unusedDependencies.forEach(unusedSelector => {
Expand All @@ -195,7 +183,7 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
if (unusedDependenciesNode) {
this.addIssue(
unusedDependenciesNode,
Rule.UNUSED_SELECTOR(unusedSelector)
Rule.UNUSED_SELECTOR(unusedSelector),
);
}
});
Expand All @@ -208,32 +196,23 @@ class NoUnusedDependenciesWalker extends Lint.RuleWalker {
const moduleName = (node.moduleSpecifier as ts.StringLiteral).text;

if (moduleName.match(/store/i)) {
addToWatchedIdentifiersFromImportNode(node, "store", true);
addToWatchedIdentifiersFromImportNode(node, 'store', true);
}

if (moduleName.match(/selectors/i)) {
addToWatchedIdentifiersFromImportNode(node, "selector");
addToWatchedIdentifiersFromImportNode(node, 'selector');
}

super.visitImportDeclaration(node);
}

public visitCallExpression(node: ts.CallExpression) {
const identifier = (node.expression as ts.Identifier).text;

if (
node.expression.kind === ts.SyntaxKind.Identifier &&
(identifier === FLUX_FACTORY.SELECT ||
identifier === FLUX_FACTORY.CONNECT)
) {
if (
identifier === FLUX_FACTORY.SELECT &&
node.parent.kind === ts.SyntaxKind.VariableDeclaration
) {
const selectorName = ((node.parent as ts.VariableDeclaration)
.name as ts.Identifier).text;
const selectMetadata = getSelectMetadata(node);
const connectMetadata = getConnectMetadata(node);

addToWatchedIdentifiers(selectorName, "selector");
if (selectMetadata || connectMetadata) {
if (selectMetadata && selectMetadata.variableName) {
addToWatchedIdentifiers(selectMetadata.variableName, 'selector');
}

this.checkDependencies(node);
Expand Down
25 changes: 11 additions & 14 deletions src/rules/selectorsFormatRule.ts
Expand Up @@ -5,8 +5,7 @@

import * as Lint from 'tslint';
import * as ts from 'typescript';

const VALID_PROPERTY_NAMES = new Set(['noMemo', 'customMemo']);
import { getSelectMetadata } from '../utils/selectUtils';

export class Rule extends Lint.Rules.AbstractRule {
public apply(sourceFile: ts.SourceFile): Array<Lint.RuleFailure> {
Expand Down Expand Up @@ -118,14 +117,18 @@ class SelectorsFormatRule extends Lint.RuleWalker {
if (isDefaultSelect && paramNode.initializer) {
this.addFailureAtNode(
paramNode,
this.getFormattedError('Default arguments are forbidden for default select with auto-memoization.'),
this.getFormattedError(
'Default arguments are forbidden for default select with auto-memoization.',
),
);
}

if (isDefaultSelect && paramNode.questionToken) {
this.addFailureAtNode(
paramNode,
this.getFormattedError('Optional arguments are forbidden for default select with auto-memoization.'),
this.getFormattedError(
'Optional arguments are forbidden for default select with auto-memoization.',
),
);
}

Expand All @@ -139,24 +142,18 @@ class SelectorsFormatRule extends Lint.RuleWalker {
}

public visitCallExpression(node: ts.CallExpression) {
const identifierExpression =
ts.isPropertyAccessExpression(node.expression) &&
VALID_PROPERTY_NAMES.has(node.expression.name.text)
? node.expression.expression
: node.expression;
const selectMetadata = getSelectMetadata(node, this.validIdentifiers);

if (
ts.isIdentifier(identifierExpression) &&
this.validIdentifiers.has(identifierExpression.text)
) {
if (selectMetadata) {
if (node.arguments.length < 2) {
return this.addFailureAtNode(
node,
this.getFormattedError('Selector must have at least 2 arguments'),
);
}

const isDefaultSelect = !ts.isPropertyAccessExpression(node.expression);
const isDefaultSelect =
!selectMetadata.hasNoMemo && !selectMetadata.hasCustomMemo;

this.checkDependenciesNode(node.arguments[0]);
this.checkResultFuncNode(node.arguments[1], isDefaultSelect);
Expand Down
47 changes: 21 additions & 26 deletions src/rules/sortFluxDependenciesRule.ts
Expand Up @@ -3,16 +3,18 @@
* All rights reserved.
*/

import * as Lint from "tslint";
import * as ts from "typescript";
import { IOptions } from "tslint";
import * as Lint from 'tslint';
import * as ts from 'typescript';
import { IOptions } from 'tslint';
import { getSelectMetadata } from '../utils/selectUtils';
import { getConnectMetadata } from '../utils/connectUtils';

export class Rule extends Lint.Rules.AbstractRule {
public static ERROR = `Dependency array is not sorted correctly!`;

public apply(sourceFile: ts.SourceFile): Array<Lint.RuleFailure> {
return this.applyWithWalker(
new SortFluxDependencies(sourceFile, this.getOptions())
new SortFluxDependencies(sourceFile, this.getOptions()),
);
}
}
Expand All @@ -25,7 +27,7 @@ const MAX_LINE_LENGTH = 80;
function getIndentation(node: ts.Node, sourceFile: ts.SourceFile): string {
const text = sourceFile.text.substr(node.pos, node.getStart() - node.pos);
const matches = text.match(/([ \t]*)$/);
return matches !== null ? matches[1] : "";
return matches !== null ? matches[1] : '';
}

class SortFluxDependencies extends Lint.RuleWalker {
Expand All @@ -39,8 +41,8 @@ class SortFluxDependencies extends Lint.RuleWalker {

const configuration = {
maxLineLength: MAX_LINE_LENGTH,
reference: "",
...options.ruleArguments[0]
reference: '',
...options.ruleArguments[0],
} as {
reference: string;
maxLineLength: number;
Expand All @@ -59,14 +61,14 @@ class SortFluxDependencies extends Lint.RuleWalker {
// let's apply this rule only on "clean" dependency array. Just the Identifiers, no spreads or magic like that.
if (
!(selectorsNode as ts.ArrayLiteralExpression).elements.every(
node => node.kind === ts.SyntaxKind.Identifier
node => node.kind === ts.SyntaxKind.Identifier,
)
) {
return;
}

const dependencyNodes = (selectorsNode as ts.ArrayLiteralExpression).elements.filter(
node => node.kind === ts.SyntaxKind.Identifier
node => node.kind === ts.SyntaxKind.Identifier,
) as Array<ts.Identifier>;

const sortedImportsIdentifiers = [...dependencyNodes]
Expand All @@ -75,19 +77,15 @@ class SortFluxDependencies extends Lint.RuleWalker {

if (!dependencyNodes[0]) return;

const indentation = getIndentation(
dependencyNodes[0],
this.getSourceFile()
);
const indentation = getIndentation(dependencyNodes[0], this.getSourceFile());

// Let's help prettier a bit to write nodes in line or in block
let formatedSortedIdentifiers;
if (
sortedImportsIdentifiers.join().length >
this.configuration.maxLineLength
sortedImportsIdentifiers.join().length > this.configuration.maxLineLength
) {
formatedSortedIdentifiers = sortedImportsIdentifiers.join(
`,\n${indentation}`
`,\n${indentation}`,
);
} else {
formatedSortedIdentifiers = sortedImportsIdentifiers.join(`, `);
Expand All @@ -96,7 +94,7 @@ class SortFluxDependencies extends Lint.RuleWalker {
const fixer = Lint.Replacement.replaceFromTo(
dependencyNodes[0].getStart(),
dependencyNodes[dependencyNodes.length - 1].getEnd(),
formatedSortedIdentifiers
formatedSortedIdentifiers,
);

for (let i = 0; i < dependencyNodes.length; i++) {
Expand All @@ -108,23 +106,20 @@ class SortFluxDependencies extends Lint.RuleWalker {
Rule.ERROR +
(this.configuration.reference
? ` ${this.configuration.reference}`
: ""),
fixer
)
: ''),
fixer,
),
);
}
}
}
}

public visitCallExpression(node: ts.CallExpression) {
const identifier = (node.expression as ts.Identifier).text;
const selectMetadata = getSelectMetadata(node);
const connectMetadata = getConnectMetadata(node);

if (
node.expression.kind === ts.SyntaxKind.Identifier &&
// support select lib and aso connect called as argument, for instance with 'compose'
(identifier === "select" || identifier === "connect")
) {
if (selectMetadata || connectMetadata) {
this.checkSelectors(node);
}

Expand Down
23 changes: 23 additions & 0 deletions src/utils/connectUtils.ts
@@ -0,0 +1,23 @@
/**
* Copyright (c) ProductBoard, Inc.
* All rights reserved.
*/

import * as ts from 'typescript';

const DEFAULT_CONNECT_NAME = 'connect';

export const getConnectMetadata = (
node: ts.CallExpression,
) => {
if (
!ts.isIdentifier(node.expression) ||
node.expression.text !== DEFAULT_CONNECT_NAME
) {
return null;
}

return {
identifierExpression: node.expression,
}
}