Skip to content

Commit

Permalink
Merge pull request #22 from JoshuaKGoldberg/walk
Browse files Browse the repository at this point in the history 
Converted to walk function instead of a RuleWalker
  • Loading branch information
@bcherny
bcherny authored Aug 20, 2018
2 parents a381698 + 687be4f commit 63b35de
Showing 1 changed file with 90 additions and 84 deletions.
174 changes: 90 additions & 84 deletions noCircularImportsRule.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { relative, sep } from 'path';
import * as Lint from 'tslint';
import { IOptions } from 'tslint/lib/language/rule/rule';
import * as ts from 'typescript';

interface Options {
/** @internal */
compilerOptions: ts.CompilerOptions

/** @internal */
rootDir: string
}

export class Rule extends Lint.Rules.TypedRule {
static FAILURE_STRING = 'circular import detected'

Expand All @@ -22,9 +29,16 @@ export class Rule extends Lint.Rules.TypedRule {
const resolvedFile = sourceFile.fileName
imports.delete(resolvedFile)

const walker = new NoCircularImportsWalker(sourceFile, this.getOptions(), program)
const compilerOptions = program.getCompilerOptions()

return this.applyWithWalker(walker)
return this.applyWithFunction(
sourceFile,
walk,
{
compilerOptions,
rootDir: compilerOptions.rootDir || process.cwd(),
},
program.getTypeChecker())
}
}

Expand All @@ -34,51 +48,44 @@ const imports = new Map<string, Map<string, ts.Node>>()
const found = new Set<string>()
const nodeModulesRe = new RegExp(`\\${sep}node_modules\\${sep}`)

class NoCircularImportsWalker extends Lint.RuleWalker {
constructor(sourceFile: ts.SourceFile, options: IOptions, private program: ts.Program) {
super(sourceFile, options)
}

visitSourceFile(sourceFile: ts.SourceFile) {
// Instead of visiting all children, this is faster. We know imports are statements anyway.
sourceFile.statements.forEach(statement => {
// export declarations seem to be missing from the current SyntaxWalker
if (ts.isExportDeclaration(statement)) {
this.visitImportOrExportDeclaration(statement)
}
else if (ts.isImportDeclaration(statement)) {
this.visitImportOrExportDeclaration(statement)
}
})

const fileName = sourceFile.fileName

// Check for cycles, remove any cycles that have been found already (otherwise we'll report
// false positive on every files that import from the real cycles, and users will be driven
// mad).
// The checkCycle is many order of magnitude faster than getCycle, but does not keep a history
// of the cycle itself. Only get the full cycle if we found one.
if (this.checkCycle(fileName)) {
const allCycles = this.getAllCycles(fileName)

for (const maybeCycle of allCycles) {
// Slice the array so we don't match this file twice.
if (maybeCycle.slice(1, -1).some(fileName => found.has(fileName))) {
continue
}
maybeCycle.forEach(x => found.add(x))
const node = imports.get(fileName) !.get(maybeCycle[1]) !

const compilerOptions = this.program.getCompilerOptions()
this.addFailureAt(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + ": " + maybeCycle
.concat(fileName)
.map(x => relative(compilerOptions.rootDir || process.cwd(), x))
.join(' -> '))
function walk(context: Lint.WalkContext<Options>) {
// Instead of visiting all children, this is faster. We know imports are statements anyway.
context.sourceFile.statements.forEach(statement => {
// export declarations seem to be missing from the current SyntaxWalker
if (ts.isExportDeclaration(statement)) {
visitImportOrExportDeclaration(statement)
}
else if (ts.isImportDeclaration(statement)) {
visitImportOrExportDeclaration(statement)
}
})

const fileName = context.sourceFile.fileName

// Check for cycles, remove any cycles that have been found already (otherwise we'll report
// false positive on every files that import from the real cycles, and users will be driven
// mad).
// The checkCycle is many order of magnitude faster than getCycle, but does not keep a history
// of the cycle itself. Only get the full cycle if we found one.
if (checkCycle(fileName)) {
const allCycles = getAllCycles(fileName)

for (const maybeCycle of allCycles) {
// Slice the array so we don't match this file twice.
if (maybeCycle.slice(1, -1).some(fileName => found.has(fileName))) {
continue
}
maybeCycle.forEach(x => found.add(x))
const node = imports.get(fileName) !.get(maybeCycle[1]) !

context.addFailureAt(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + ": " + maybeCycle
.concat(fileName)
.map(x => relative(context.options.rootDir, x))
.join(' -> '))
}
}

visitImportOrExportDeclaration(node: ts.ImportDeclaration | ts.ExportDeclaration) {
function visitImportOrExportDeclaration(node: ts.ImportDeclaration | ts.ExportDeclaration) {
if (!node.parent || !ts.isSourceFile(node.parent)) {
return
}
Expand All @@ -91,9 +98,8 @@ class NoCircularImportsWalker extends Lint.RuleWalker {
return
}
const importFileName = node.moduleSpecifier.text
const compilerOptions = this.program.getCompilerOptions()

const resolved = ts.resolveModuleName(importFileName, fileName, compilerOptions, ts.sys)
const resolved = ts.resolveModuleName(importFileName, fileName, context.options.compilerOptions, ts.sys)
if (!resolved || !resolved.resolvedModule) {
return
}
Expand All @@ -105,55 +111,55 @@ class NoCircularImportsWalker extends Lint.RuleWalker {
return
}

this.addToGraph(fileName, resolvedImportFileName, node)
addToGraph(fileName, resolvedImportFileName, node)
}
}

private addToGraph(thisFileName: string, importCanonicalName: string, node: ts.Node) {
let i = imports.get(thisFileName)
if (!i) {
imports.set(thisFileName, i = new Map)
}
i.set(importCanonicalName, node)
function addToGraph(thisFileName: string, importCanonicalName: string, node: ts.Node) {
let i = imports.get(thisFileName)
if (!i) {
imports.set(thisFileName, i = new Map)
}
i.set(importCanonicalName, node)
}

private checkCycle(moduleName: string): boolean {
const accumulator = new Set<string>()

const moduleImport = imports.get(moduleName)
if (!moduleImport)
return false
function checkCycle(moduleName: string): boolean {
const accumulator = new Set<string>()

const toCheck = Array.from(moduleImport.keys())
for (let i = 0; i < toCheck.length; i++) {
const current = toCheck[i]
if (current == moduleName) {
return true
}
accumulator.add(current)
const moduleImport = imports.get(moduleName)
if (!moduleImport)
return false

toCheck.push(
...Array.from((imports.get(current) || new Map).keys())
.filter(i => !accumulator.has(i))
)
const toCheck = Array.from(moduleImport.keys())
for (let i = 0; i < toCheck.length; i++) {
const current = toCheck[i]
if (current == moduleName) {
return true
}
accumulator.add(current)

return false
toCheck.push(
...Array.from((imports.get(current) || new Map).keys())
.filter(i => !accumulator.has(i))
)
}

private getAllCycles(moduleName: string, accumulator: string[] = []): string[][] {
const moduleImport = imports.get(moduleName)
if (!moduleImport) return []
if (accumulator.indexOf(moduleName) !== -1)
return [accumulator]
return false
}

const all: string[][] = []
for (const imp of Array.from(moduleImport.keys())) {
const c = this.getAllCycles(imp, accumulator.concat(moduleName))
function getAllCycles(moduleName: string, accumulator: string[] = []): string[][] {
const moduleImport = imports.get(moduleName)
if (!moduleImport) return []
if (accumulator.indexOf(moduleName) !== -1)
return [accumulator]

if (c.length)
all.push(...c)
}
const all: string[][] = []
for (const imp of Array.from(moduleImport.keys())) {
const c = getAllCycles(imp, accumulator.concat(moduleName))

return all
if (c.length)
all.push(...c)
}

return all
}

0 comments on commit 63b35de

Please sign in to comment.