Skip to content

Commit

Permalink
feature(typescript): detect typescript dependencies that only exist b…
Browse files Browse the repository at this point in the history
…efore compile time (sverweij#30)

* feature(typescript): use the typescript AST to infer some of the dependencies:
Some typescript features and dependencies get lost when the typescript compiler translates to javascript. By using the typescript AST instead of the javascript-AST-after-translation (e.g. when you depend only on a type from a module, if you depend on a module that isn't used (yet), have triple-slash directives, or use the import-from-export-equals construct) , we'll be able to catch these as well. Will solve issue sverweij#28

* refactor(extract): pull out the 'parse' step from extract.js and ...
  * ... rename all modules that extract dependencies from an AST from extract-<modulesystem> to etract-<modulesystem>-deps as that better conveys their purpose
  * ... put the extract-yadda-deps in a separate folder
  * ... separate parsing and extracting in the (new) typescript dependency extraction
* style(depcruise): updates some of our own dependency-cruiser rules
* test(extract): add a test for when dependencies aren't used and they still should show up in the output
* doc(parse): document the parser wrappers
* doc(samples): updates the tslint sample
  • Loading branch information
sverweij authored Jan 28, 2018
1 parent efa5b28 commit b4abad7
Show file tree
Hide file tree
Showing 29 changed files with 661 additions and 120 deletions.
6 changes: 3 additions & 3 deletions .dependency-cruiser-custom.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"name": "restrict-fs-access",
"comment": "restrict file access to a few modules only",
"severity": "error",
"from": { "path": "^src/(report|validate|main|extract/transpile)" },
"from": { "pathNot": "^src/(extract/parse|extract/resolve|extract/gatherInitialSources\\.js|cli)|^test" },
"to": { "path": "^fs$" }
}, {
"name": "no-deprecated-core",
Expand All @@ -30,10 +30,10 @@
"to": { "path": "^test/[^\\/]+/.+", "pathNot": "utl|$1[^\\.]+\\.json$"}
}, {
"name": "prefer-no-lodash",
"comment": "We want to minimize the dependency on lodash a bit - so flag dependencies that go there",
"comment": "We want to minimize the dependency on lodash a bit - so flag dependencies that go there and include it as a whole",
"severity": "info",
"from": {},
"to": { "path": "lodash"}
"to": { "path": "lodash\\.js"}
}, {
"name": "no-dep-on-test",
"severity": "error",
Expand Down
141 changes: 88 additions & 53 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,42 @@ src/extract/addValidations.js: \
src/validate/index.js

src/extract/extract.js: \
src/extract/extract-AMD.js \
src/extract/extract-ES6.js \
src/extract/extract-commonJS.js \
src/extract/ast-extractors/extract-AMD-deps.js \
src/extract/ast-extractors/extract-ES6-deps.js \
src/extract/ast-extractors/extract-commonJS-deps.js \
src/extract/ast-extractors/extract-typescript-deps.js \
src/extract/ignore.js \
src/extract/resolve/index.js \
src/extract/parse/toJavascriptAST.js \
src/extract/parse/toTypescriptAST.js \
src/extract/resolve/index.js

src/extract/ast-extractors/extract-AMD-deps.js: \
src/extract/ast-extractors/extract-commonJS-deps.js

src/extract/parse/toJavascriptAST.js: \
src/extract/transpile/index.js

src/extract/extract-AMD.js: \
src/extract/extract-commonJS.js
src/extract/transpile/index.js: \
src/extract/transpile/meta.js

src/extract/transpile/meta.js: \
package.json \
src/extract/transpile/coffeeWrap.js \
src/extract/transpile/javaScriptWrap.js \
src/extract/transpile/liveScriptWrap.js \
src/extract/transpile/typeScriptWrap.js

src/extract/transpile/coffeeWrap.js: \
package.json

src/extract/transpile/liveScriptWrap.js: \
package.json

src/extract/transpile/typeScriptWrap.js: \
package.json

src/extract/parse/toTypescriptAST.js: \
package.json

src/extract/resolve/index.js: \
src/extract/resolve/resolve-AMD.js \
Expand All @@ -188,25 +215,6 @@ src/extract/resolve/resolve-commonJS.js: \
src/extract/transpile/meta.js \
src/utl/pathToPosix.js

src/extract/transpile/meta.js: \
package.json \
src/extract/transpile/coffeeWrap.js \
src/extract/transpile/javaScriptWrap.js \
src/extract/transpile/liveScriptWrap.js \
src/extract/transpile/typeScriptWrap.js

src/extract/transpile/coffeeWrap.js: \
package.json

src/extract/transpile/liveScriptWrap.js: \
package.json

src/extract/transpile/typeScriptWrap.js: \
package.json

src/extract/transpile/index.js: \
src/extract/transpile/meta.js

src/extract/gatherInitialSources.js: \
src/extract/ignore.js \
src/extract/transpile/meta.js
Expand All @@ -233,14 +241,17 @@ src/main/ruleSet/validate.js: \
ALL_SRC=src/main/index.js \
package.json \
src/extract/addValidations.js \
src/extract/ast-extractors/extract-AMD-deps.js \
src/extract/ast-extractors/extract-ES6-deps.js \
src/extract/ast-extractors/extract-commonJS-deps.js \
src/extract/ast-extractors/extract-typescript-deps.js \
src/extract/dependencyEndsUpAtFrom.js \
src/extract/extract-AMD.js \
src/extract/extract-ES6.js \
src/extract/extract-commonJS.js \
src/extract/extract.js \
src/extract/gatherInitialSources.js \
src/extract/ignore.js \
src/extract/index.js \
src/extract/parse/toJavascriptAST.js \
src/extract/parse/toTypescriptAST.js \
src/extract/resolve/determineDependencyTypes.js \
src/extract/resolve/index.js \
src/extract/resolve/localNpmHelpers.js \
Expand Down Expand Up @@ -303,15 +314,42 @@ src/extract/addValidations.js: \
src/validate/index.js

src/extract/extract.js: \
src/extract/extract-AMD.js \
src/extract/extract-ES6.js \
src/extract/extract-commonJS.js \
src/extract/ast-extractors/extract-AMD-deps.js \
src/extract/ast-extractors/extract-ES6-deps.js \
src/extract/ast-extractors/extract-commonJS-deps.js \
src/extract/ast-extractors/extract-typescript-deps.js \
src/extract/ignore.js \
src/extract/resolve/index.js \
src/extract/parse/toJavascriptAST.js \
src/extract/parse/toTypescriptAST.js \
src/extract/resolve/index.js

src/extract/ast-extractors/extract-AMD-deps.js: \
src/extract/ast-extractors/extract-commonJS-deps.js

src/extract/parse/toJavascriptAST.js: \
src/extract/transpile/index.js

src/extract/extract-AMD.js: \
src/extract/extract-commonJS.js
src/extract/transpile/index.js: \
src/extract/transpile/meta.js

src/extract/transpile/meta.js: \
package.json \
src/extract/transpile/coffeeWrap.js \
src/extract/transpile/javaScriptWrap.js \
src/extract/transpile/liveScriptWrap.js \
src/extract/transpile/typeScriptWrap.js

src/extract/transpile/coffeeWrap.js: \
package.json

src/extract/transpile/liveScriptWrap.js: \
package.json

src/extract/transpile/typeScriptWrap.js: \
package.json

src/extract/parse/toTypescriptAST.js: \
package.json

src/extract/resolve/index.js: \
src/extract/resolve/resolve-AMD.js \
Expand All @@ -333,25 +371,6 @@ src/extract/resolve/resolve-commonJS.js: \
src/extract/transpile/meta.js \
src/utl/pathToPosix.js

src/extract/transpile/meta.js: \
package.json \
src/extract/transpile/coffeeWrap.js \
src/extract/transpile/javaScriptWrap.js \
src/extract/transpile/liveScriptWrap.js \
src/extract/transpile/typeScriptWrap.js

src/extract/transpile/coffeeWrap.js: \
package.json

src/extract/transpile/liveScriptWrap.js: \
package.json

src/extract/transpile/typeScriptWrap.js: \
package.json

src/extract/transpile/index.js: \
src/extract/transpile/meta.js

src/extract/gatherInitialSources.js: \
src/extract/ignore.js \
src/extract/transpile/meta.js
Expand Down Expand Up @@ -404,6 +423,22 @@ test/cli/normalizeOptions.spec.js: \
test/cli/validateFileExistence.spec.js: \
src/cli/validateFileExistence.js

test/extract/ast-extractors/extract-typescript-commonjs.spec.js: \
test/extract/ast-extractors/extract-typescript.utl.js

test/extract/ast-extractors/extract-typescript.utl.js: \
src/extract/ast-extractors/extract-typescript-deps.js \
src/extract/parse/toTypescriptAST.js

test/extract/ast-extractors/extract-typescript-exports.spec.js: \
test/extract/ast-extractors/extract-typescript.utl.js

test/extract/ast-extractors/extract-typescript-imports.spec.js: \
test/extract/ast-extractors/extract-typescript.utl.js

test/extract/ast-extractors/extract-typescript-triple-slash-directives.spec.js: \
test/extract/ast-extractors/extract-typescript.utl.js

test/extract/dependencyEndsUpAtFrom.spec.js: \
src/extract/dependencyEndsUpAtFrom.js

Expand Down
8 changes: 5 additions & 3 deletions doc/real-world-samples.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,19 @@ For coloring strings in the terminal. A typical _Sorhus style_ micro module that
It is possible to use dependency-cruiser to infer dependencies of typescript
projects.

We got the picture of tslint below by changing to the tslint/src folder and
running this:
We got the picture of tslint by running this in its source folder:
```sh
dependency-cruise -T dot -x node_modules index.ts | dot -T png > tslint-without-node_modules.png
dependency-cruise -T dot -x node_modules -v -- src/index.ts | dot -T png > tslint-without-node_modules.png
```

(Yep, that's all - no separate transpilation steps necessary ...)

### tslint
[palantir/tslint](https://github.com/palantir/tslint) - linter for typescript.

The orange lines are warnings for circular dependencies, which occur around two of
tslint's 'barrel' `index.ts` modules:

![tslint](real-world-samples/tslint-without-node_modules.png)


Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified doc/real-world-samples/tslint-without-node_modules.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "dependency-cruiser",
"version": "2.11.1",
"version": "2.12.0-beta-2",
"description": "Validate and visualize dependencies. With your rules. JavaScript, TypeScript, CoffeeScript. ES6, CommonJS, AMD.",
"bin": {
"dependency-cruiser": "bin/dependency-cruise",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

const walk = require('acorn/dist/walk');
const extractCommonJSDependencies = require("./extract-commonJS");
const extractCommonJSDependencies = require("./extract-commonJS-deps");

module.exports = (pAST, pDependencies) => {
walk.simple(
Expand Down
File renamed without changes.
File renamed without changes.
71 changes: 71 additions & 0 deletions src/extract/ast-extractors/extract-typescript-deps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"use strict";

/*
* Both extractImport* assume the imports/ exports can only occur at
* top level. AFAIK this the only place they're allowed, so we should
* be good. Otherwise we'll need to walk the tree.
*/
function extractImportsAndExports(pAST) {
const IMPORT_DECLARATION_KIND = 238;
const EXPORT_DECLARATION_KIND = 244;

return pAST.statements
.filter(pStatement =>
(
pStatement.kind === IMPORT_DECLARATION_KIND ||
pStatement.kind === EXPORT_DECLARATION_KIND
) &&
Boolean(pStatement.moduleSpecifier)
).map(pStatement => ({
moduleName: pStatement.moduleSpecifier.text,
moduleSystem: 'es6'
}));
}

function extractImportEquals(pAST) {
const IMPORT_EQUALS_DECLARATION_KIND = 237;

return pAST.statements
.filter(pStatement =>
pStatement.kind === IMPORT_EQUALS_DECLARATION_KIND
).map(pStatement => ({
moduleName: pStatement.moduleReference.expression.text,
moduleSystem: 'cjs'
}));
}

/**
* might be wise to distinguish the three types of /// directive that
* can come out of this as the resolution algorithm might differ
*
* @param {*} pAST - typescript syntax tree
* @returns {object} - 'tripple slash' dependencies
*/
function extractTrippleSlashDirectives(pAST) {
return pAST.referencedFiles.map(
pReference => ({
moduleName: pReference.fileName,
moduleSystem: 'tsd'
})
).concat(
pAST.typeReferenceDirectives.map(
pReference => ({
moduleName: pReference.fileName,
moduleSystem: 'tsd'
})
)
).concat(
pAST.amdDependencies.map(
pReference => ({
moduleName: pReference.path,
moduleSystem: 'tsd'
})
)
);
}

module.exports = (pTypeScriptAST) =>
extractImportsAndExports(pTypeScriptAST)
.concat(extractImportEquals(pTypeScriptAST))
.concat(extractTrippleSlashDirectives(pTypeScriptAST));

Loading

0 comments on commit b4abad7

Please sign in to comment.