Skip to content

Commit 5160f58

Browse files
devversionjelbourn
devversion
authored and
jelbourn
committed
chore(ci): forbidden identifiers check all source files (#968)
* fix(button): remove relative import from other scope. * Removes invalid relative import from other scope. * Forbidden identifiers should run against all sources when not checking a PR. * Cleanup Forbidden identifiers script, by using ES6 arrow functions. Fixes #967.
1 parent e28734b commit 5160f58

File tree

1 file changed

+144
-141
lines changed

1 file changed

+144
-141
lines changed

‎scripts/ci/forbidden-identifiers.js

Lines changed: 144 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,21 @@
11
#!/usr/bin/env node
22

33
'use strict';
4+
45
/*
5-
* This script analyzes the current commits of the CI.
6-
* It will search for blocked statements, which have been added in the commits and fail if present.
6+
* The forbidden identifiers script will check for blocked statements and also detect invalid
7+
* imports of other scope packages.
8+
*
9+
* When running against a PR, the script will only analyze the specific amount of commits inside
10+
* of the Pull Request.
11+
*
12+
* By default it checks all source files and fail if any errors were found.
713
*/
814

915
const child_process = require('child_process');
1016
const fs = require('fs');
1117
const path = require('path');
12-
13-
const exec = function(cmd) {
14-
return new Promise(function(resolve, reject) {
15-
child_process.exec(cmd, function(err, stdout /*, stderr */) {
16-
if (err) {
17-
reject(err);
18-
} else {
19-
resolve(stdout);
20-
}
21-
});
22-
});
23-
};
24-
18+
const glob = require('glob').sync;
2519

2620
const blocked_statements = [
2721
'\\bddescribe\\(',
@@ -34,64 +28,123 @@ const blocked_statements = [
3428
'from \\\'rxjs/Rx\\\'',
3529
];
3630

37-
// Retrieves all scope packages dynamically from the source.
38-
// Developers shouldn't be able to import from other scope packages by using relative paths.
39-
const componentFolders = fs
40-
.readdirSync(`${__dirname}/../../src/components`)
41-
.map(componentName => `src/components/${componentName}`);
42-
43-
const scopePackages = ['src/core'].concat(componentFolders);
44-
31+
const sourceFolders = ['./src', './e2e'];
32+
const scopePackages = ['src/core'].concat(glob('src/components/*'));
4533
const blockedRegex = new RegExp(blocked_statements.join('|'), 'g');
4634
const importRegex = /from\s+'(.*)';/g;
4735

48-
/**
49-
* Find the fork point between HEAD of the current branch, and master.
50-
* @return {Promise<string>} A promise which resolves with the fork SHA (or reject).
36+
/*
37+
* Verify that the current PR is not adding any forbidden identifiers.
38+
* Run the forbidden identifiers check against all sources when not verifying a PR.
5139
*/
52-
function findForkPoint() {
53-
return exec('git merge-base --fork-point HEAD master')
54-
.then(function(stdout) {
55-
return stdout.split('\n')[0];
56-
});
57-
}
40+
41+
findTestFiles()
42+
43+
/* Only match .js or .ts, and remove .d.ts files. */
44+
.then(files => files.filter(name => /\.(js|ts)$/.test(name) && !/\.d\.ts$/.test(name)))
45+
46+
/* Read content of the filtered files */
47+
.then(files => files.map(name => ({ name, content: fs.readFileSync(name, 'utf-8') })))
48+
49+
/* Run checks against content of the filtered files. */
50+
.then(diffList => {
51+
52+
return diffList.reduce((errors, diffFile) => {
53+
let fileName = diffFile.name;
54+
let content = diffFile.content.split('\n');
55+
let lineCount = 0;
56+
57+
content.forEach(line => {
58+
lineCount++;
59+
60+
let matches = line.match(blockedRegex);
61+
let scopeImport = isRelativeScopeImport(fileName, line);
62+
63+
if (matches || scopeImport) {
64+
65+
let error = {
66+
fileName: fileName,
67+
lineNumber: lineCount,
68+
statement: matches && matches[0] || scopeImport.invalidStatement
69+
};
70+
71+
if (scopeImport) {
72+
error.messages = [
73+
'You are using an import statement, which imports a file from another scope package.',
74+
`Please import the file by using the following path: ${scopeImport.scopeImportPath}`
75+
];
76+
}
77+
78+
errors.push(error);
79+
}
80+
});
81+
82+
return errors;
83+
84+
}, []);
85+
})
86+
87+
/* Print the resolved errors to the console */
88+
.then(errors => {
89+
if (errors.length > 0) {
90+
console.error('Error: You are using one or more blocked statements:\n');
91+
92+
errors.forEach(entry => {
93+
if (entry.messages) {
94+
entry.messages.forEach(message => console.error(` ${message}`))
95+
}
96+
97+
console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${entry.statement}.\n`);
98+
});
99+
100+
process.exit(1);
101+
}
102+
})
103+
104+
.catch(err => {
105+
// An error occurred in this script. Output the error and the stack.
106+
console.error('An error occurred during execution:');
107+
console.error(err.stack || err);
108+
process.exit(2);
109+
});
110+
58111

59112
/**
60-
* Get the commit range to evaluate when this script is run.
61-
* @return {Promise<string>} A commit range of the form ref1...ref2.
113+
* Resolves all files, which should run against the forbidden identifiers check.
114+
* @return {Promise.<Array.<string>>} Files to be checked.
62115
*/
63-
function getCommitRange() {
64-
if (process.env['TRAVIS_COMMIT_RANGE']) {
65-
return Promise.resolve(process.env['TRAVIS_COMMIT_RANGE']);
66-
} else {
67-
return findForkPoint().then((forkPointSha) => `${forkPointSha}...HEAD`);
116+
function findTestFiles() {
117+
if (process.env['TRAVIS_PULL_REQUEST']) {
118+
return findChangedFiles();
68119
}
120+
121+
var files = sourceFolders.map(folder => {
122+
return glob(`${folder}/**/*.ts`);
123+
}).reduce((files, fileSet) => files.concat(fileSet), []);
124+
125+
return Promise.resolve(files);
69126
}
70127

71128
/**
72129
* List all the files that have been changed or added in the last commit range.
73-
* @returns {Promise<Array<string>>} Resolves with a list of files that are
74-
* added or changed.
130+
* @returns {Promise.<Array.<string>>} Resolves with a list of files that are added or changed.
75131
*/
76132
function findChangedFiles() {
77-
return getCommitRange()
78-
.then(range => {
79-
return exec(`git diff --name-status ${range} ./src ./e2e`);
80-
})
133+
let commitRange = process.env['TRAVIS_COMMIT_RANGE'];
134+
135+
return exec(`git diff --name-status ${commitRange} ${sourceFolders.join(' ')}`)
81136
.then(rawDiff => {
82-
// Ignore deleted files.
83-
return rawDiff.split('\n')
84-
.filter(function(line) {
85-
// Status: C## => Copied (##% confident)
86-
// R## => Renamed (##% confident)
87-
// D => Deleted
88-
// M => Modified
89-
// A => Added
90-
return line.match(/([CR][0-9]*|[AM])\s+/);
91-
})
92-
.map(function(line) {
93-
return line.split(/\s+/, 2)[1];
94-
});
137+
return rawDiff
138+
.split('\n')
139+
.filter(line => {
140+
// Status: C## => Copied (##% confident)
141+
// R## => Renamed (##% confident)
142+
// D => Deleted
143+
// M => Modified
144+
// A => Added
145+
return line.match(/([CR][0-9]*|[AM])\s+/);
146+
})
147+
.map(line => line.split(/\s+/, 2)[1]);
95148
});
96149
}
97150

@@ -125,103 +178,53 @@ function isRelativeScopeImport(fileName, line) {
125178
// Transform the relative import path into an absolute path.
126179
importPath = path.resolve(path.dirname(fileName), importPath);
127180

128-
let currentScope = findScope(fileName);
181+
let fileScope = findScope(fileName);
129182
let importScope = findScope(importPath);
130183

131-
if (currentScope !== importScope) {
132-
// Transforms the invalid import statement into a correct import statement, which uses the scope package.
133-
let scopeImport = `@angular2-material/${path.basename(importScope)}/${path.relative(importScope, importPath)}`;
184+
if (fileScope.path !== importScope.path) {
185+
186+
// Creates a valid import statement, which uses the correct scope package.
187+
let importFilePath = path.relative(importScope.path, importPath);
188+
let validImportPath = `@angular2-material/${importScope.name}/${importFilePath}`;
134189

135190
return {
136-
currentScope: currentScope,
137-
importScope: importScope,
191+
fileScope: fileScope.name,
192+
importScope: importScope.name,
138193
invalidStatement: importMatch[0],
139-
scopeImportPath: scopeImport
194+
scopeImportPath: validImportPath
140195
}
141196
}
142197

143-
return false;
144-
145198
function findScope(filePath) {
146-
// Normalize the filePath as well, to avoid issues with the different environments path delimiter.
199+
// Normalize the filePath, to avoid issues with the different environments path delimiter.
147200
filePath = path.normalize(filePath);
148201

149-
// Iterate through all scopes and try to find them inside of the file path.
150-
return scopePackages.filter(scope => filePath.indexOf(path.normalize(scope)) !== -1).pop();
202+
// Iterate through all scope paths and try to find them inside of the file path.
203+
var scopePath = scopePackages
204+
.filter(scope => filePath.indexOf(path.normalize(scope)) !== -1)
205+
.pop();
206+
207+
// Return an object containing the name of the scope and the associated path.
208+
return {
209+
name: path.basename(scopePath),
210+
path: scopePath
211+
}
151212
}
152213

153214
}
154215

155-
156-
// Find all files, check for errors, and output every errors found.
157-
findChangedFiles()
158-
.then(fileList => {
159-
// Only match .js or .ts, and remove .d.ts files.
160-
return fileList.filter(function(name) {
161-
return name.match(/\.[jt]s$/) && !name.match(/\.d\.ts$/);
216+
/**
217+
* Executes a process command and wraps it inside of a promise.
218+
* @returns {Promise.<String>}
219+
*/
220+
function exec(cmd) {
221+
return new Promise(function(resolve, reject) {
222+
child_process.exec(cmd, function(err, stdout /*, stderr */) {
223+
if (err) {
224+
reject(err);
225+
} else {
226+
resolve(stdout);
227+
}
162228
});
163-
})
164-
.then(fileList => {
165-
// Read every file and return a Promise that will contain an array of
166-
// Object of the form { fileName, content }.
167-
return Promise.all(fileList.map(function(fileName) {
168-
return {
169-
fileName: fileName,
170-
content: fs.readFileSync(fileName, { encoding: 'utf-8' })
171-
};
172-
}));
173-
})
174-
.then(diffList => {
175-
// Reduce the diffList to an array of errors. The array will be empty if no errors
176-
// were found.
177-
return diffList.reduce((errors, diffEntry) => {
178-
let fileName = diffEntry.fileName;
179-
let content = diffEntry.content.split('\n');
180-
let ln = 0;
181-
182-
// Get all the lines that start with `+`.
183-
content.forEach(function(line) {
184-
ln++;
185-
186-
let matches = line.match(blockedRegex);
187-
let isScopeImport = isRelativeScopeImport(fileName, line);
188-
189-
if (matches || isScopeImport) {
190-
// Accumulate all errors at once.
191-
let error = {
192-
fileName: fileName,
193-
lineNumber: ln,
194-
statement: matches && matches[0] || isScopeImport.invalidStatement
195-
};
196-
197-
if (isScopeImport) {
198-
error.message =
199-
' You are using an import statement, which imports a file from another scope package.\n' +
200-
` Please import the file by using the following path: ${isScopeImport.scopeImportPath}`;
201-
}
202-
203-
errors.push(error);
204-
}
205-
});
206-
return errors;
207-
}, []);
208-
})
209-
.then(errors => {
210-
if (errors.length > 0) {
211-
console.error('Error: You are using a statement in your commit, which is not allowed.\n');
212-
213-
errors.forEach(entry => {
214-
if (entry.message) console.error(entry.message);
215-
console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${entry.statement}.\n`);
216-
});
217-
218-
process.exit(1);
219-
}
220-
})
221-
.catch(err => {
222-
// An error occured in this script. Output the error and the stack.
223-
console.error('An error occured during execution:');
224-
console.error(err);
225-
console.error(err.stack);
226-
process.exit(2);
227229
});
230+
}

0 commit comments

Comments
 (0)