mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
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.
This commit is contained in:
Paul Gschwendtner
committed by
Jeremy Elbourn
Jeremy Elbourn
parent
e28734b54b
commit
5160f58371
+144
-141
@@ -1,27 +1,21 @@
|
||||
#!/usr/bin/env node
|
||||
|
||||
'use strict';
|
||||
|
||||
/*
|
||||
* This script analyzes the current commits of the CI.
|
||||
* It will search for blocked statements, which have been added in the commits and fail if present.
|
||||
* The forbidden identifiers script will check for blocked statements and also detect invalid
|
||||
* imports of other scope packages.
|
||||
*
|
||||
* When running against a PR, the script will only analyze the specific amount of commits inside
|
||||
* of the Pull Request.
|
||||
*
|
||||
* By default it checks all source files and fail if any errors were found.
|
||||
*/
|
||||
|
||||
const child_process = require('child_process');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const exec = function(cmd) {
|
||||
return new Promise(function(resolve, reject) {
|
||||
child_process.exec(cmd, function(err, stdout /*, stderr */) {
|
||||
if (err) {
|
||||
reject(err);
|
||||
} else {
|
||||
resolve(stdout);
|
||||
}
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
const glob = require('glob').sync;
|
||||
|
||||
const blocked_statements = [
|
||||
'\\bddescribe\\(',
|
||||
@@ -34,64 +28,123 @@ const blocked_statements = [
|
||||
'from \\\'rxjs/Rx\\\'',
|
||||
];
|
||||
|
||||
// Retrieves all scope packages dynamically from the source.
|
||||
// Developers shouldn't be able to import from other scope packages by using relative paths.
|
||||
const componentFolders = fs
|
||||
.readdirSync(`${__dirname}/../../src/components`)
|
||||
.map(componentName => `src/components/${componentName}`);
|
||||
|
||||
const scopePackages = ['src/core'].concat(componentFolders);
|
||||
|
||||
const sourceFolders = ['./src', './e2e'];
|
||||
const scopePackages = ['src/core'].concat(glob('src/components/*'));
|
||||
const blockedRegex = new RegExp(blocked_statements.join('|'), 'g');
|
||||
const importRegex = /from\s+'(.*)';/g;
|
||||
|
||||
/**
|
||||
* Find the fork point between HEAD of the current branch, and master.
|
||||
* @return {Promise<string>} A promise which resolves with the fork SHA (or reject).
|
||||
/*
|
||||
* Verify that the current PR is not adding any forbidden identifiers.
|
||||
* Run the forbidden identifiers check against all sources when not verifying a PR.
|
||||
*/
|
||||
function findForkPoint() {
|
||||
return exec('git merge-base --fork-point HEAD master')
|
||||
.then(function(stdout) {
|
||||
return stdout.split('\n')[0];
|
||||
});
|
||||
}
|
||||
|
||||
findTestFiles()
|
||||
|
||||
/* Only match .js or .ts, and remove .d.ts files. */
|
||||
.then(files => files.filter(name => /\.(js|ts)$/.test(name) && !/\.d\.ts$/.test(name)))
|
||||
|
||||
/* Read content of the filtered files */
|
||||
.then(files => files.map(name => ({ name, content: fs.readFileSync(name, 'utf-8') })))
|
||||
|
||||
/* Run checks against content of the filtered files. */
|
||||
.then(diffList => {
|
||||
|
||||
return diffList.reduce((errors, diffFile) => {
|
||||
let fileName = diffFile.name;
|
||||
let content = diffFile.content.split('\n');
|
||||
let lineCount = 0;
|
||||
|
||||
content.forEach(line => {
|
||||
lineCount++;
|
||||
|
||||
let matches = line.match(blockedRegex);
|
||||
let scopeImport = isRelativeScopeImport(fileName, line);
|
||||
|
||||
if (matches || scopeImport) {
|
||||
|
||||
let error = {
|
||||
fileName: fileName,
|
||||
lineNumber: lineCount,
|
||||
statement: matches && matches[0] || scopeImport.invalidStatement
|
||||
};
|
||||
|
||||
if (scopeImport) {
|
||||
error.messages = [
|
||||
'You are using an import statement, which imports a file from another scope package.',
|
||||
`Please import the file by using the following path: ${scopeImport.scopeImportPath}`
|
||||
];
|
||||
}
|
||||
|
||||
errors.push(error);
|
||||
}
|
||||
});
|
||||
|
||||
return errors;
|
||||
|
||||
}, []);
|
||||
})
|
||||
|
||||
/* Print the resolved errors to the console */
|
||||
.then(errors => {
|
||||
if (errors.length > 0) {
|
||||
console.error('Error: You are using one or more blocked statements:\n');
|
||||
|
||||
errors.forEach(entry => {
|
||||
if (entry.messages) {
|
||||
entry.messages.forEach(message => console.error(` ${message}`))
|
||||
}
|
||||
|
||||
console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${entry.statement}.\n`);
|
||||
});
|
||||
|
||||
process.exit(1);
|
||||
}
|
||||
})
|
||||
|
||||
.catch(err => {
|
||||
// An error occurred in this script. Output the error and the stack.
|
||||
console.error('An error occurred during execution:');
|
||||
console.error(err.stack || err);
|
||||
process.exit(2);
|
||||
});
|
||||
|
||||
|
||||
/**
|
||||
* Get the commit range to evaluate when this script is run.
|
||||
* @return {Promise<string>} A commit range of the form ref1...ref2.
|
||||
* Resolves all files, which should run against the forbidden identifiers check.
|
||||
* @return {Promise.<Array.<string>>} Files to be checked.
|
||||
*/
|
||||
function getCommitRange() {
|
||||
if (process.env['TRAVIS_COMMIT_RANGE']) {
|
||||
return Promise.resolve(process.env['TRAVIS_COMMIT_RANGE']);
|
||||
} else {
|
||||
return findForkPoint().then((forkPointSha) => `${forkPointSha}...HEAD`);
|
||||
function findTestFiles() {
|
||||
if (process.env['TRAVIS_PULL_REQUEST']) {
|
||||
return findChangedFiles();
|
||||
}
|
||||
|
||||
var files = sourceFolders.map(folder => {
|
||||
return glob(`${folder}/**/*.ts`);
|
||||
}).reduce((files, fileSet) => files.concat(fileSet), []);
|
||||
|
||||
return Promise.resolve(files);
|
||||
}
|
||||
|
||||
/**
|
||||
* List all the files that have been changed or added in the last commit range.
|
||||
* @returns {Promise<Array<string>>} Resolves with a list of files that are
|
||||
* added or changed.
|
||||
* @returns {Promise.<Array.<string>>} Resolves with a list of files that are added or changed.
|
||||
*/
|
||||
function findChangedFiles() {
|
||||
return getCommitRange()
|
||||
.then(range => {
|
||||
return exec(`git diff --name-status ${range} ./src ./e2e`);
|
||||
})
|
||||
let commitRange = process.env['TRAVIS_COMMIT_RANGE'];
|
||||
|
||||
return exec(`git diff --name-status ${commitRange} ${sourceFolders.join(' ')}`)
|
||||
.then(rawDiff => {
|
||||
// Ignore deleted files.
|
||||
return rawDiff.split('\n')
|
||||
.filter(function(line) {
|
||||
// Status: C## => Copied (##% confident)
|
||||
// R## => Renamed (##% confident)
|
||||
// D => Deleted
|
||||
// M => Modified
|
||||
// A => Added
|
||||
return line.match(/([CR][0-9]*|[AM])\s+/);
|
||||
})
|
||||
.map(function(line) {
|
||||
return line.split(/\s+/, 2)[1];
|
||||
});
|
||||
return rawDiff
|
||||
.split('\n')
|
||||
.filter(line => {
|
||||
// Status: C## => Copied (##% confident)
|
||||
// R## => Renamed (##% confident)
|
||||
// D => Deleted
|
||||
// M => Modified
|
||||
// A => Added
|
||||
return line.match(/([CR][0-9]*|[AM])\s+/);
|
||||
})
|
||||
.map(line => line.split(/\s+/, 2)[1]);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -125,103 +178,53 @@ function isRelativeScopeImport(fileName, line) {
|
||||
// Transform the relative import path into an absolute path.
|
||||
importPath = path.resolve(path.dirname(fileName), importPath);
|
||||
|
||||
let currentScope = findScope(fileName);
|
||||
let fileScope = findScope(fileName);
|
||||
let importScope = findScope(importPath);
|
||||
|
||||
if (currentScope !== importScope) {
|
||||
// Transforms the invalid import statement into a correct import statement, which uses the scope package.
|
||||
let scopeImport = `@angular2-material/${path.basename(importScope)}/${path.relative(importScope, importPath)}`;
|
||||
if (fileScope.path !== importScope.path) {
|
||||
|
||||
// Creates a valid import statement, which uses the correct scope package.
|
||||
let importFilePath = path.relative(importScope.path, importPath);
|
||||
let validImportPath = `@angular2-material/${importScope.name}/${importFilePath}`;
|
||||
|
||||
return {
|
||||
currentScope: currentScope,
|
||||
importScope: importScope,
|
||||
fileScope: fileScope.name,
|
||||
importScope: importScope.name,
|
||||
invalidStatement: importMatch[0],
|
||||
scopeImportPath: scopeImport
|
||||
scopeImportPath: validImportPath
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
|
||||
function findScope(filePath) {
|
||||
// Normalize the filePath as well, to avoid issues with the different environments path delimiter.
|
||||
// Normalize the filePath, to avoid issues with the different environments path delimiter.
|
||||
filePath = path.normalize(filePath);
|
||||
|
||||
// Iterate through all scopes and try to find them inside of the file path.
|
||||
return scopePackages.filter(scope => filePath.indexOf(path.normalize(scope)) !== -1).pop();
|
||||
// Iterate through all scope paths and try to find them inside of the file path.
|
||||
var scopePath = scopePackages
|
||||
.filter(scope => filePath.indexOf(path.normalize(scope)) !== -1)
|
||||
.pop();
|
||||
|
||||
// Return an object containing the name of the scope and the associated path.
|
||||
return {
|
||||
name: path.basename(scopePath),
|
||||
path: scopePath
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
// Find all files, check for errors, and output every errors found.
|
||||
findChangedFiles()
|
||||
.then(fileList => {
|
||||
// Only match .js or .ts, and remove .d.ts files.
|
||||
return fileList.filter(function(name) {
|
||||
return name.match(/\.[jt]s$/) && !name.match(/\.d\.ts$/);
|
||||
/**
|
||||
* Executes a process command and wraps it inside of a promise.
|
||||
* @returns {Promise.<String>}
|
||||
*/
|
||||
function exec(cmd) {
|
||||
return new Promise(function(resolve, reject) {
|
||||
child_process.exec(cmd, function(err, stdout /*, stderr */) {
|
||||
if (err) {
|
||||
reject(err);
|
||||
} else {
|
||||
resolve(stdout);
|
||||
}
|
||||
});
|
||||
})
|
||||
.then(fileList => {
|
||||
// Read every file and return a Promise that will contain an array of
|
||||
// Object of the form { fileName, content }.
|
||||
return Promise.all(fileList.map(function(fileName) {
|
||||
return {
|
||||
fileName: fileName,
|
||||
content: fs.readFileSync(fileName, { encoding: 'utf-8' })
|
||||
};
|
||||
}));
|
||||
})
|
||||
.then(diffList => {
|
||||
// Reduce the diffList to an array of errors. The array will be empty if no errors
|
||||
// were found.
|
||||
return diffList.reduce((errors, diffEntry) => {
|
||||
let fileName = diffEntry.fileName;
|
||||
let content = diffEntry.content.split('\n');
|
||||
let ln = 0;
|
||||
|
||||
// Get all the lines that start with `+`.
|
||||
content.forEach(function(line) {
|
||||
ln++;
|
||||
|
||||
let matches = line.match(blockedRegex);
|
||||
let isScopeImport = isRelativeScopeImport(fileName, line);
|
||||
|
||||
if (matches || isScopeImport) {
|
||||
// Accumulate all errors at once.
|
||||
let error = {
|
||||
fileName: fileName,
|
||||
lineNumber: ln,
|
||||
statement: matches && matches[0] || isScopeImport.invalidStatement
|
||||
};
|
||||
|
||||
if (isScopeImport) {
|
||||
error.message =
|
||||
' You are using an import statement, which imports a file from another scope package.\n' +
|
||||
` Please import the file by using the following path: ${isScopeImport.scopeImportPath}`;
|
||||
}
|
||||
|
||||
errors.push(error);
|
||||
}
|
||||
});
|
||||
return errors;
|
||||
}, []);
|
||||
})
|
||||
.then(errors => {
|
||||
if (errors.length > 0) {
|
||||
console.error('Error: You are using a statement in your commit, which is not allowed.\n');
|
||||
|
||||
errors.forEach(entry => {
|
||||
if (entry.message) console.error(entry.message);
|
||||
console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${entry.statement}.\n`);
|
||||
});
|
||||
|
||||
process.exit(1);
|
||||
}
|
||||
})
|
||||
.catch(err => {
|
||||
// An error occured in this script. Output the error and the stack.
|
||||
console.error('An error occured during execution:');
|
||||
console.error(err);
|
||||
console.error(err.stack);
|
||||
process.exit(2);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user