Skip to content

Commit

Permalink
chore(forbidden-identifiers): detect incorrect line endings (#1722)
Browse files Browse the repository at this point in the history 
* chore(forbidden-identifiers): detect incorrect line endings

* Removes stale scope checking (from `@angular2-material/$COMPONENT` times)
* Adds support for error grouping (useful when having a file full of invalid line endings)
* Moves chains into exta functions to understand procession easily.

* Update comment wording
  • Loading branch information
@devversion @tinayuangao
devversion authored and tinayuangao committed Nov 29, 2016
1 parent cb3bb7a commit f1b9b2c
Showing 1 changed file with 85 additions and 116 deletions.
201 changes: 85 additions & 116 deletions scripts/ci/forbidden-identifiers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ const blocked_statements = [
'\\bxit\\(',
'\\bdebugger;',
'from \\\'rxjs/Rx\\\'',
'\\r'
];

const sourceFolders = ['./src', './e2e'];
const libRoot = 'src/lib';
const blockedRegex = new RegExp(blocked_statements.join('|'), 'g');
const importRegex = /from\s+'(.*)';/g;

/*
* Verify that the current PR is not adding any forbidden identifiers.
Expand All @@ -47,69 +46,105 @@ findTestFiles()
.then(files => files.map(name => ({ name, content: fs.readFileSync(name, 'utf-8') })))

/* Run checks against content of the filtered files. */
.then(diffList => {
.then(diffList => findErrors(diffList))

return diffList.reduce((errors, diffFile) => {
let fileName = diffFile.name;
let content = diffFile.content.split('\n');
let lineCount = 0;
/* Groups similar errors to simplify console output */
.then(errors => groupErrors(errors))

content.forEach(line => {
lineCount++;
/* Print the resolved errors to the console */
.then(errors => printErrors(errors))

.catch(err => {
// An error occurred in this script. Output the error and the stack.
console.error(err.stack || err);
process.exit(2);
});

/**
* Finds errors inside of changed files by running them against the blocked statements.
* @param {{name: string, content: string}[]} diffList
*/
function findErrors(diffList) {
return diffList.reduce((errors, diffFile) => {
let fileName = diffFile.name;
let content = diffFile.content.split('\n');
let lineNumber = 0;

let matches = line.match(blockedRegex);
let scopeImport = path.relative(libRoot, fileName).startsWith('..')
? isRelativeScopeImport(fileName, line)
: false;
content.forEach(line => {
lineNumber++;

if (matches || scopeImport) {
let matches = line.match(blockedRegex);

let error = {
fileName: fileName,
lineNumber: lineCount,
statement: matches && matches[0] || scopeImport.invalidStatement
};
if (matches) {

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({
fileName,
lineNumber,
statement: matches[0]
});

errors.push(error);
}
});
}
});

return errors;
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');
/**
* Groups similar errors in the same file which are present over a range of lines.
* @param {{fileName: string, lineNumber: number, statement: string}[]} errors
*/
function groupErrors(errors) {

errors.forEach(entry => {
if (entry.messages) {
entry.messages.forEach(message => console.error(` ${message}`))
}
let initialError, initialLine, previousLine;

console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${entry.statement}.\n`);
});
return errors.filter(error => {

process.exit(1);
if (initialError && isSimilarError(error)) {
previousLine = error.lineNumber;

// Overwrite the lineNumber with a string, which indicates the range of lines.
initialError.lineNumber = `${initialLine}-${previousLine}`;

return false;
}
})

.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);
initialLine = previousLine = error.lineNumber;
initialError = error;

return true;
});

/** Checks whether the given error is similar to the previous one. */
function isSimilarError(error) {
return initialError.fileName === error.fileName &&
initialError.statement === error.statement &&
previousLine === (error.lineNumber - 1);
}
}

/**
* Prints all errors to the console and terminates the process if errors were present.
* @param {{fileName: string, lineNumber: number, statement: string}[]} errors
*/
function printErrors(errors) {
if (errors.length > 0) {

console.error('Error: You are using one or more blocked statements:\n');

errors.forEach(entry => {

// Stringify the statement to represent line-endings or other unescaped characters.
let statement = JSON.stringify(entry.statement);

console.error(` ${entry.fileName}@${entry.lineNumber}, Statement: ${statement}.\n`);
});

// Exit the process with an error exit code to notify the CI.
process.exit(1);
}
}

/**
* Resolves all files, which should run against the forbidden identifiers check.
Expand All @@ -120,8 +155,8 @@ function findTestFiles() {
return findChangedFiles();
}

var files = sourceFolders.map(folder => {
return glob(`${folder}/**/*.ts`);
let files = sourceFolders.map(folder => {
return glob(`${folder}/**/*`);
}).reduce((files, fileSet) => files.concat(fileSet), []);

return Promise.resolve(files);
Expand Down Expand Up @@ -150,72 +185,6 @@ function findChangedFiles() {
});
}

/**
* Checks the line for any relative imports of a scope package, which should be imported by using
* the scope package name instead of the relative path.
* @param fileName Filename to validate the path
* @param line Line to be checked.
*/
function isRelativeScopeImport(fileName, line) {
if (fileName.startsWith(libRoot)) {
return false;
}

let importMatch = importRegex.exec(line);

// We have to reset the last index of the import regex, otherwise we
// would have incorrect matches in the next execution.
importRegex.lastIndex = 0;

// Skip the check if the current line doesn't match any imports.
if (!importMatch) {
return false;
}

let importPath = importMatch[1];

// Skip the check when the import doesn't start with a dot, because the line
// isn't importing any file relatively. Also applies to scope packages starting
// with `@`.
if (!importPath.startsWith('.')) {
return false;
}

// Transform the relative import path into an absolute path.
importPath = path.resolve(path.dirname(fileName), importPath);

let fileScope = findScope(fileName);
let importScope = findScope(importPath);

if (fileScope.path !== importScope.path) {

// Creates a valid import statement, which uses the correct scope package.
let validImportPath = `@angular/material`;

return {
fileScope: fileScope.name,
importScope: importScope.name,
invalidStatement: importMatch[0],
scopeImportPath: validImportPath
}
}

function findScope(filePath) {
// Normalize the filePath, to avoid issues with the different environments path delimiter.
filePath = path.normalize(filePath);

// Iterate through all scope paths and try to find them inside of the file path.
var scopePath = filePath.indexOf(path.normalize(libRoot)) == -1 ? libRoot : filePath;

// Return an object containing the name of the scope and the associated path.
return {
name: path.basename(scopePath),
path: scopePath
}
}

}

/**
* Executes a process command and wraps it inside of a promise.
* @returns {Promise.<String>}
Expand Down

0 comments on commit f1b9b2c

Please sign in to comment.