From ac0166020e4a19ccad8b51a38c904c4678745d3b Mon Sep 17 00:00:00 2001 From: Karl-Philipp Wulfert Date: Fri, 24 May 2019 17:53:55 +0200 Subject: [PATCH] refactor: adjust code to stricter rules --- src/cli.ts | 12 ++- src/common.ts | 245 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 188 insertions(+), 69 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 21e5068f..5182c9b8 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -37,7 +37,10 @@ const currentWorkingDirectory = cwd(); // configure commander commander - .version(JSON.parse(readFileSync(resolve(__dirname, '..', 'package.json')).toString()).version) + .version(JSON.parse( + readFileSync(resolve(__dirname, '..', 'package.json')) + .toString(), + ).version) .option('-p, --path ', `Path of project to add files to (${currentWorkingDirectory})`, currentWorkingDirectory) .option('-r, --replace', 'Whether to replace existing files or not', false) .parse(process.argv); @@ -54,7 +57,8 @@ if (!existsSync(resolve(path, 'package.json'))) { const packageJsonPath = resolve(path, 'package.json'); // read package.json in provided path -const packageJson = JSON.parse(readFileSync(packageJsonPath).toString()); +const packageJson = JSON.parse(readFileSync(packageJsonPath) + .toString()); // check if provided path is this package if (packageJson.name === '@openstapps/configuration') { @@ -93,8 +97,10 @@ checkCIConfig(rules, path); checkCopyrightYears(path, 'src'); +const indentation = 2; + if (packageJsonChanged) { - writeFileSync(resolve(path, 'package.json'), JSON.stringify(packageJson, null, 2)); + writeFileSync(resolve(path, 'package.json'), JSON.stringify(packageJson, null, indentation)); consoleLog(`Changes were written to '${packageJsonPath}'.`); } diff --git a/src/common.ts b/src/common.ts index 6640689d..a67dd41b 100644 --- a/src/common.ts +++ b/src/common.ts @@ -1,3 +1,17 @@ +/* + * Copyright (C) 2018, 2019 StApps + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ import chalk from 'chalk'; import {execSync} from 'child_process'; import {copyFileSync, existsSync, lstatSync, PathLike, readdirSync, readFileSync} from 'fs'; @@ -6,6 +20,73 @@ import {satisfies, valid} from 'semver'; import {isDeepStrictEqual} from 'util'; import {parse, stringify} from 'yaml'; +interface NYCConfiguration { + [prop: string]: boolean | number | string[]; +} + +interface PackageJSONPerson { + /** + * Email of the author + */ + email?: string; + /** + * Name of the author + */ + name: string; + /** + * URL of the author + */ + url?: string; +} + +interface PackageJSON { + /** + * Author of the package + */ + author: string | PackageJSONPerson; + + /** + * Contributors of the package + */ + contributors: Array; + + /** + * Dependencies + */ + dependencies?: { + [dependency: string]: string; + }; + + /** + * Development dependencies + */ + devDependencies?: { + [devDependency: string]: string; + }; + + /** + * License + */ + license: string; + + /** + * NYC configuration + */ + nyc: NYCConfiguration; + + /** + * Openstapps configuration + */ + openstappsConfiguration: Configuration; + + /** + * Scripts + */ + scripts: { + [name: string]: string; + }; +} + /** * Configuration for the configuration check */ @@ -47,7 +128,7 @@ export interface Rules { /** * Expected CI config */ - ciConfig: any; + ciConfig: { [k: string]: string | object; }; /** * Expected dependencies */ @@ -67,7 +148,7 @@ export interface Rules { /** * Expected NYC configuration */ - nycConfiguration: any; + nycConfiguration: NYCConfiguration; /** * Expected scripts */ @@ -82,7 +163,7 @@ export interface Rules { export function consoleInfo(...args: string[]): void { args.forEach((arg) => { /* tslint:disable-next-line:no-console */ - console.info('\n' + chalk.cyan(arg)); + console.info(`\n${chalk.cyan(arg)}`); }); } @@ -95,7 +176,7 @@ export function consoleWarn(...args: string[]): void { args.forEach((arg) => { const lines = arg.split('\n'); /* tslint:disable-next-line:no-console */ - console.warn('\n' + chalk.red.bold(lines[0])); + console.warn(`\n${chalk.red.bold(lines[0])}`); for (const line of lines.slice(1)) { /* tslint:disable-next-line:no-console */ console.info(line); @@ -111,7 +192,7 @@ export function consoleWarn(...args: string[]): void { export function consoleLog(...args: string[]): void { args.forEach((arg) => { /* tslint:disable-next-line:no-console */ - console.log('\n' + chalk.green.bold(arg)); + console.log(`\n${chalk.green.bold(arg)}`); }); } @@ -121,43 +202,56 @@ export function consoleLog(...args: string[]): void { * @param rules Rules for check * @param packageJson package.json to check dependencies in */ -export function checkDependencies(rules: Rules, packageJson: any): void { - for (const dependency of rules.dependencies) { - const [name, version] = dependency.split(':'); - const installedVersion = packageJson.dependencies[name]; +export function checkDependencies(rules: Rules, packageJson: PackageJSON): void { + if (typeof packageJson.dependencies === 'object') { + for (const dependency of rules.dependencies) { + const [name, version] = dependency.split(':'); + const installedVersion = packageJson.dependencies[name]; - if (typeof packageJson.dependencies === 'undefined' || typeof packageJson.dependencies[name] === 'undefined') { - consoleWarn(`Dependency '${name}' is missing. + if (typeof packageJson.dependencies === 'undefined' || typeof packageJson.dependencies[name] === 'undefined') { + consoleWarn(`Dependency '${name}' is missing. Please install with 'npm install --save-exact ${name}'.`); - } else if ( - typeof version !== 'undefined' - && valid(version) - && !satisfies(installedVersion, version) - ) { - consoleWarn( - `Version '${installedVersion}' of dependency '${name} does not satisfy constraint '${version}'.`, - ); + } else if ( + typeof version !== 'undefined' + && typeof valid(version) === 'string' + && !satisfies(installedVersion, version) + ) { + consoleWarn( + `Version '${installedVersion}' of dependency '${name} does not satisfy constraint '${version}'.`, + ); + } } } - for (const devDependency of rules.devDependencies) { - const [name, version] = devDependency.split(':'); - const installedVersion = packageJson.dependencies[name]; + if (typeof packageJson.devDependencies === 'object') { + for (const devDependency of rules.devDependencies) { + const [devName, devVersion] = devDependency.split(':'); + let installedVersion = packageJson.devDependencies[devName]; + if (typeof packageJson.dependencies === 'object') { + const [name] = devDependency.split(':'); + if (typeof packageJson.devDependencies[name] === 'string') { + installedVersion = packageJson.dependencies[name]; + } + } - if ( - typeof packageJson.dependencies === 'undefined' || typeof packageJson.dependencies[name] === 'undefined' - && typeof packageJson.devDependencies === 'undefined' || typeof packageJson.devDependencies[name] === 'undefined' - ) { - consoleWarn(`Dev dependency '${name}' is missing. -Please install with 'npm install --save-exact --save-dev ${name}'.`); - } else if ( - typeof version !== 'undefined' - && valid(version) - && !satisfies(installedVersion, version) - ) { - consoleWarn( - `Version '${installedVersion}' of dev dependency '${name} does not satisfy constraint '${version}'.`, - ); + if ( + (typeof packageJson.dependencies === 'undefined' || typeof packageJson.dependencies[devName] === 'undefined') + && ( + typeof packageJson.devDependencies === 'undefined' + || typeof packageJson.devDependencies[devName] === 'undefined' + ) + ) { + consoleWarn(`Dev dependency '${devName}' is missing. +Please install with 'npm install --save-exact --save-dev ${devName}'.`); + } else if ( + typeof devVersion !== 'undefined' + && typeof valid(devVersion) === 'string' + && !satisfies(installedVersion, devVersion) + ) { + consoleWarn( + `Version '${installedVersion}' of dev dependency '${devName} does not satisfy constraint '${devVersion}'.`, + ); + } } } } @@ -174,7 +268,8 @@ export function checkConfigurationFilesAreExtended(path: string): void { const expectedPath = `./node_modules/@openstapps/configuration/${file}`; if (existsSync(fileToCheck)) { - const configFile = JSON.parse(readFileSync(fileToCheck).toString()); + const configFile = JSON.parse(readFileSync(fileToCheck) + .toString()); const configFileExtended = ( typeof configFile.extends === 'string' @@ -190,7 +285,7 @@ export function checkConfigurationFilesAreExtended(path: string): void { consoleWarn(`File '${fileToCheck}' should extend '${expectedPath}'! Example: -${readFileSync(resolve(__dirname, '..', 'templates', 'template-' + file))}`); +${readFileSync(resolve(__dirname, '..', 'templates', `template-${file}`))}`); } } }); @@ -208,12 +303,14 @@ export function checkNeededFiles(rules: Rules, path: string, replaceFlag: boolea let suggestOverwrite = false; // copy needed files - rules.files.forEach((file) => { + for (let file of rules.files) { let destinationFile = file; // remove templates directory for destination files if (destinationFile.indexOf('templates') === 0) { - destinationFile = destinationFile.split(sep).slice(1).join(sep); + destinationFile = destinationFile.split(sep) + .slice(1) + .join(sep); file = join('templates', `template-${destinationFile}`); } @@ -225,7 +322,8 @@ export function checkNeededFiles(rules: Rules, path: string, replaceFlag: boolea copyFileSync(source, destination); consoleInfo(`Copied file '${source}' to '${destination}'.`); } else if (destinationFile === '.npmignore') { - const npmIgnore = readFileSync(destination).toString(); + const npmIgnore = readFileSync(destination) + .toString(); const ignoredPatterns = npmIgnore.split('\n'); @@ -256,7 +354,7 @@ https://gitlab.com/openstapps/configuration/issues/11`); suggestOverwrite = true; } } - }); + } return suggestOverwrite; } @@ -267,7 +365,7 @@ https://gitlab.com/openstapps/configuration/issues/11`); * @param rules Rules for check * @param packageJson package.json to check license in */ -export function checkLicenses(rules: Rules, packageJson: any): void { +export function checkLicenses(rules: Rules, packageJson: PackageJSON): void { // check if license is one of the expected ones if (rules.licenses.indexOf(packageJson.license) === -1) { consoleWarn(`License should be one of '${rules.licenses.join(', ')}'!`); @@ -282,12 +380,14 @@ export function checkLicenses(rules: Rules, packageJson: any): void { * @param replaceFlag Whether or not to replace NYC configuration * @return Whether or not package.json was changed and if overwrite is suggested */ -export function checkNYCConfiguration(rules: Rules, packageJson: any, replaceFlag: boolean): [boolean, boolean] { +export function checkNYCConfiguration(rules: Rules, packageJson: PackageJSON, replaceFlag: boolean) + : [boolean, boolean] { let packageJsonChanged = false; let suggestOverwrite = false; // check if nyc is a dependency - if (typeof packageJson.devDependencies === 'object' && Object.keys(packageJson.devDependencies).indexOf('nyc') >= 0) { + if (typeof packageJson.devDependencies === 'object' && Object.keys(packageJson.devDependencies) + .indexOf('nyc') >= 0) { if (typeof packageJson.nyc === 'undefined' || replaceFlag) { // add NYC configuration packageJson.nyc = rules.nycConfiguration; @@ -313,7 +413,7 @@ export function checkNYCConfiguration(rules: Rules, packageJson: any, replaceFla * @param replaceFlag Whether or not to replace scripts * @return Whether or not the package.json was changed */ -export function checkScripts(rules: Rules, packageJson: any, replaceFlag: boolean): boolean { +export function checkScripts(rules: Rules, packageJson: PackageJSON, replaceFlag: boolean): boolean { let packageJsonChanged = false; // check if scripts is a map @@ -323,7 +423,11 @@ export function checkScripts(rules: Rules, packageJson: any, replaceFlag: boolea packageJsonChanged = true; } - Object.keys(rules.scripts).forEach((scriptName) => { + for (const scriptName in rules.scripts) { + if (!rules.scripts.hasOwnProperty(scriptName)) { + continue; + } + const scriptToCheck = packageJson.scripts[scriptName]; // check if script exists @@ -337,7 +441,7 @@ export function checkScripts(rules: Rules, packageJson: any, replaceFlag: boolea consoleWarn(`Script '${scriptName}' in 'package.json' should be: '${rules.scripts[scriptName].replace('\n', '\\n')}'.`); } - }); + } return packageJsonChanged; } @@ -348,25 +452,26 @@ export function checkScripts(rules: Rules, packageJson: any, replaceFlag: boolea * @param path Path to directory * @param packageJson package.json to check contributors in */ -export function checkContributors(path: PathLike, packageJson: any): void { +export function checkContributors(path: PathLike, packageJson: PackageJSON): void { const execBuffer = execSync(`git --git-dir=${path}/.git --work-tree=${path} log --format=\'%aN\' | sort -u`); - for (let author of execBuffer.toString().split('\n')) { - author = author.trim(); + for (let person of execBuffer.toString() + .split('\n')) { + person = person.trim(); - if (author === '') { + if (person === '') { continue; } let authorIsAttributed = false; authorIsAttributed = authorIsAttributed - || (typeof packageJson.author === 'string' && packageJson.author.indexOf(author) >= 0) - || (Array.isArray(packageJson.contributors) && packageJson.contributors.find((contributor: string) => { - return contributor.indexOf(author) >= 0; - })); + || (typeof packageJson.author === 'string' && packageJson.author.indexOf(person) >= 0) + || (Array.isArray(packageJson.contributors) && packageJson.contributors.findIndex((contributor) => { + return typeof contributor === 'string' && contributor.indexOf(person) >= 0; + }) >= 0); if (!authorIsAttributed) { - consoleWarn(`'${author}' should be attributed as author or contributor.`); + consoleWarn(`'${person}' should be attributed as author or contributor.`); } } } @@ -393,7 +498,7 @@ export function checkCIConfig(rules: Rules, path: string): void { } if (!isDeepStrictEqual(rules.ciConfig[entry], ciConfig[entry])) { - const completeEntry: any = {}; + const completeEntry: { [k: string]: string | object; } = {}; completeEntry[entry] = rules.ciConfig[entry]; consoleWarn(`Entry '${entry}' in '${pathToCiConfig}' is incorrect. Expected value is: ${stringify(completeEntry)}`); @@ -428,11 +533,13 @@ export function checkCopyrightYears(path: PathLike, subDir: PathLike): void { .split('\n') .map((date) => parseInt(date.split('-')[0], 10)) .filter((year) => { - if (seen.indexOf(year) >= 0 || !year.toString().match(/[0-9]{4}/)) { + const stringYear = year.toString(); + if (seen.indexOf(year) >= 0 || stringYear.match(/[0-9]{4}/) !== null) { return false; } seen.push(year); + return true; }) .sort(); @@ -440,13 +547,16 @@ export function checkCopyrightYears(path: PathLike, subDir: PathLike): void { const fileStats = lstatSync(fileSystemObjectPath); if (fileStats.isFile()) { - const content = readFileSync(fileSystemObjectPath).toString().split('\n'); + const content = readFileSync(fileSystemObjectPath) + .toString() + .split('\n'); - let copyrightYearsString: string = ''; + let copyrightYearsString = ''; for (const line of content) { const match = line.match(/^ \* Copyright \(C\) ([0-9\-,\s]*) StApps$/); + const expectedMatchLength = 2; - if (Array.isArray(match) && match.length === 2) { + if (Array.isArray(match) && match.length === expectedMatchLength) { copyrightYearsString = match[1]; } } @@ -454,7 +564,8 @@ export function checkCopyrightYears(path: PathLike, subDir: PathLike): void { if (copyrightYearsString === '') { consoleWarn(`Copyright line for file '${fileSystemObjectPath}' could not be found!`); } else { - const copyrightYearsWithIntervals = copyrightYearsString.split(',').map((year) => year.trim()); + const copyrightYearsWithIntervals = copyrightYearsString.split(',') + .map((year) => year.trim()); const copyrightYears: number[] = []; for (const copyrightYear of copyrightYearsWithIntervals) { @@ -497,7 +608,7 @@ export function checkCopyrightYears(path: PathLike, subDir: PathLike): void { * * @param packageJson package.json to get configuration from */ -export function getConfiguration(packageJson: any): Configuration { +export function getConfiguration(packageJson: PackageJSON): Configuration { const defaultConfiguration: Configuration = { forPackaging: true, hasCli: true, @@ -569,7 +680,7 @@ export function getRules(configuration: Configuration): Rules { // expected scripts const scripts: { [k: string]: string; } = { /* tslint:disable-next-line:max-line-length */ - 'changelog': 'conventional-changelog -p angular -i CHANGELOG.md -s -r 0 && git add CHANGELOG.md && git commit -m \'docs: update changelog\'', + changelog: 'conventional-changelog -p angular -i CHANGELOG.md -s -r 0 && git add CHANGELOG.md && git commit -m \'docs: update changelog\'', 'check-configuration': 'openstapps-configuration', tslint: 'tslint -p tsconfig.json -c tslint.json \'src/**/*.ts\'', }; @@ -581,7 +692,7 @@ export function getRules(configuration: Configuration): Rules { ]; // expected values in CI config - const ciConfig: { [k: string]: any; } = { + const ciConfig = { /* tslint:disable:object-literal-sort-keys */ image: 'registry.gitlab.com/openstapps/projectmanagement/node', cache: { @@ -660,6 +771,8 @@ export function getRules(configuration: Configuration): Rules { }; for (const ignoreCiEntry of configuration.ignoreCiEntries) { + // tslint:disable-next-line:ban-ts-ignore + // @ts-ignore delete ciConfig[ignoreCiEntry]; }