Skip to content

chore(): forbidden identifiers check for relative scope imports. #764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 85 additions & 6 deletions scripts/ci/forbidden-identifiers.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

const child_process = require('child_process');
const fs = require('fs');
const path = require('path');

const exec = function(cmd) {
return new Promise(function(resolve, reject) {
Expand All @@ -33,7 +34,16 @@ 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 blockedRegex = new RegExp(blocked_statements.join('|'), 'g');
const importRegex = /from\s+'(.*)';/g;

/**
* Find the fork point between HEAD of the current branch, and master.
Expand Down Expand Up @@ -85,6 +95,63 @@ 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) {
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 currentScope = 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)}`;

return {
currentScope: currentScope,
importScope: importScope,
invalidStatement: importMatch[0],
scopeImportPath: scopeImport
}
}

return false;

function findScope(filePath) {
// Normalize the filePath as well, 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();
}

}


// Find all files, check for errors, and output every errors found.
findChangedFiles()
Expand Down Expand Up @@ -116,23 +183,35 @@ findChangedFiles()
content.forEach(function(line) {
ln++;

let m = line.match(blockedRegex);
if (m) {
let matches = line.match(blockedRegex);
let isScopeImport = isRelativeScopeImport(fileName, line);

if (matches || isScopeImport) {
// Accumulate all errors at once.
errors.push({
let error = {
fileName: fileName,
lineNumber: ln,
statement: m[0]
});
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.');
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`);
});

Expand Down