From 010d4368ec0ffe56c33a7803f22132f6acdf8a32 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 27 Jun 2016 16:49:21 +0200 Subject: [PATCH 1/2] chore(): forbidden identifiers check for relative scope imports. * Checks the added imports for any relative scope imports and automatically suggests the correct Scope Import statement. --- scripts/ci/forbidden-identifiers.js | 91 +++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/scripts/ci/forbidden-identifiers.js b/scripts/ci/forbidden-identifiers.js index d406cab92f4f..a950b5a3c90f 100755 --- a/scripts/ci/forbidden-identifiers.js +++ b/scripts/ci/forbidden-identifiers.js @@ -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) { @@ -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 '(.*)'/g; /** * Find the fork point between HEAD of the current branch, and master. @@ -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() @@ -116,14 +183,24 @@ 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; @@ -131,8 +208,10 @@ findChangedFiles() }) .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`); }); From e9b4da605821118b670d3e4381c94cf8ec993510 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 7 Jul 2016 07:21:17 +0200 Subject: [PATCH 2/2] Import regex should check for multiple whitespaces and semi-colon at end --- scripts/ci/forbidden-identifiers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/forbidden-identifiers.js b/scripts/ci/forbidden-identifiers.js index a950b5a3c90f..150ca74e626c 100755 --- a/scripts/ci/forbidden-identifiers.js +++ b/scripts/ci/forbidden-identifiers.js @@ -43,7 +43,7 @@ const componentFolders = fs const scopePackages = ['src/core'].concat(componentFolders); const blockedRegex = new RegExp(blocked_statements.join('|'), 'g'); -const importRegex = /from '(.*)'/g; +const importRegex = /from\s+'(.*)';/g; /** * Find the fork point between HEAD of the current branch, and master.