Skip to content

Commit

Permalink
chore: Add rule to guarantee node runtime compatibility
Browse files Browse the repository at this point in the history
This adds a rule to make sure the codebase is all configured to support
node runtimes complying with `>= 8.11.0`, by ensuring that the
configured `@types/node` version starts with `^8.`.

In order to allow applying this rule to the top-level `package.json`, a
feature was added to `pkglint` that allows users to confgure `include`
and `exclude` filters as part of their `package.json`'s `pkglint` setup.
Exclusions always have precedence over inclusions, and specifying
`disable: true` has the same effect as specifying `exclude: '*'`. Both
the `include` and `exclude` values can contain patterns that use `*` as
a wild-card that maches any number of characters (including no
character at all), and can be either a single pattern, or an array of
patterns. `include` defaults to `*`.

Fixes #1194
  • Loading branch information
RomainMuller committed Nov 19, 2018
1 parent 52b7554 commit 7b3d8cd
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 66 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"name": "aws-cdk",
"pkglint": {
"ignore": true
"include": "dependencies/node-version"
},
"scripts": {
"pkglint": "tools/pkglint/bin/pkglint -f ."
},
"devDependencies": {
"@types/node": "^8.10.36",
"@types/node": "^8.10.38",
"@types/nodeunit": "^0.0.30",
"conventional-changelog-cli": "^2.0.5",
"lerna": "^3.3.0",
Expand Down
19 changes: 7 additions & 12 deletions tools/pkglint/bin/pkglint.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env node
import path = require('path');
import yargs = require('yargs');
import { findPackageJsons, PackageJson, ValidationRule } from '../lib';
import { findPackageJsons, ValidationRule } from '../lib';

// tslint:disable:no-shadowed-variable
const argv = yargs
Expand All @@ -11,24 +12,22 @@ const argv = yargs
// Our version of yargs doesn't support positional arguments yet
argv.directory = argv._[0];

if (argv.directory == null) {
argv.directory = ".";
}
argv.directory = path.resolve(argv.directory || '.', process.cwd());

async function main(): Promise<void> {
const ruleClasses = require('../lib/rules');
const rules: ValidationRule[] = Object.keys(ruleClasses).map(key => new ruleClasses[key]()).filter(obj => obj instanceof ValidationRule);

const pkgs = findPackageJsons(argv.directory).filter(shouldIncludePackage);
const pkgs = findPackageJsons(argv.directory);

rules.forEach(rule => pkgs.forEach(pkg => rule.prepare(pkg)));
rules.forEach(rule => pkgs.forEach(pkg => rule.validate(pkg)));
rules.forEach(rule => pkgs.filter(pkg => pkg.shouldApply(rule)).forEach(pkg => rule.prepare(pkg)));
rules.forEach(rule => pkgs.filter(pkg => pkg.shouldApply(rule)).forEach(pkg => rule.validate(pkg)));

if (argv.fix) {
pkgs.forEach(pkg => pkg.applyFixes());
}

pkgs.forEach(pkg => pkg.displayReports());
pkgs.forEach(pkg => pkg.displayReports(argv.directory));

if (pkgs.some(p => p.hasReports)) {
throw new Error('Some package.json files had errors');
Expand All @@ -40,7 +39,3 @@ main().catch((e) => {
console.error(e);
process.exit(1);
});

function shouldIncludePackage(pkg: PackageJson) {
return !(pkg.json.pkglint && pkg.json.pkglint.ignore);
}
78 changes: 73 additions & 5 deletions tools/pkglint/lib/packagejson.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import colors = require('colors/safe');
import fs = require('fs-extra');
import path = require('path');

Expand Down Expand Up @@ -27,7 +28,8 @@ export function findPackageJsons(root: string): PackageJson[] {
if (file !== 'node_modules' && (fs.lstatSync(fullPath)).isDirectory()) {
recurse(fullPath);
}
} }
}
}

recurse(root);
return ret;
Expand All @@ -36,6 +38,8 @@ export function findPackageJsons(root: string): PackageJson[] {
export type Fixer = () => void;

export interface Report {
ruleName: string;

message: string;

fix?: Fixer;
Expand All @@ -45,15 +49,40 @@ export interface Report {
* Class representing a package.json file and the issues we found with it
*/
export class PackageJson {
public readonly json: any;
public readonly json: { [key: string]: any };
public readonly packageRoot: string;
public readonly packageName: string;

private readonly includeRules: RegExp[];
private readonly excludeRules: RegExp[];

private reports: Report[] = [];

constructor(public readonly fullPath: string) {
this.json = JSON.parse(fs.readFileSync(fullPath, { encoding: 'utf-8' }));
this.packageRoot = path.dirname(path.resolve(fullPath));
this.packageName = this.json.name;

const disabled = this.json.pkglint && this.json.pkglint.ignore;
this.includeRules = _forceArray(this.json.pkglint && this.json.pkglint.include) || [/^.*$/];
this.excludeRules = _forceArray(this.json.pkglint && this.json.pkglint.exclude) || (disabled ? [/^.*$/] : []);

function _forceArray(arg: string | string[] | undefined): RegExp[] | undefined {
if (arg == null) { return arg; }
if (Array.isArray(arg)) { return arg.map(_toRegExp); }
return [_toRegExp(arg)];
}

function _toRegExp(pattern: string): RegExp {
pattern = pattern.split('*').map(s => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('.*');
return new RegExp(`^${pattern}$`);
}
}

public shouldApply(rule: ValidationRule): boolean {
const included = this.includeRules.find(r => r.test(rule.name)) != null;
const excluded = this.excludeRules.find(r => r.test(rule.name)) != null;
return included && !excluded;
}

public save() {
Expand Down Expand Up @@ -93,10 +122,12 @@ export class PackageJson {
this.reports = nonFixable;
}

public displayReports() {
public displayReports(relativeTo: string) {
if (this.hasReports) {
process.stderr.write(`${path.resolve(this.fullPath)}\n`);
this.reports.forEach(report => process.stderr.write(`- ${report.message}\n`));
process.stderr.write(`In package ${colors.blue(path.relative(relativeTo, this.fullPath))}\n`);
this.reports.forEach(report => {
process.stderr.write(`- [${colors.yellow(report.ruleName)}] ${report.message}${report.fix ? colors.green(' (fixable)') : ''}\n`);
});
}
}

Expand Down Expand Up @@ -159,6 +190,14 @@ export class PackageJson {
return key !== undefined ? deps[key] : undefined;
}

/**
* @param predicate the predicate to select dependencies to be extracted
* @returns the list of dependencies matching a pattern.
*/
public getDependencies(predicate: (s: string) => boolean): Array<{ name: string, version: string }> {
return Object.keys(this.json.dependencies || {}).filter(predicate).map(name => ({ name, version: this.json.dependencies[name] }));
}

/**
* Adds a devDependency to the package.
*/
Expand All @@ -170,6 +209,17 @@ export class PackageJson {
this.json.devDependencies[module] = version;
}

/**
* Adds a dependency to the package.
*/
public addDependency(module: string, version = '*') {
if (!('dependencies' in this.json)) {
this.json.dependencies = {};
}

this.json.dependencies[module] = version;
}

public removeDevDependency(moduleOrPredicate: ((s: string) => boolean) | string) {
if (!('devDependencies' in this.json)) {
return;
Expand All @@ -186,6 +236,22 @@ export class PackageJson {
}
}

public removeDependency(moduleOrPredicate: ((s: string) => boolean) | string) {
if (!('dependencies' in this.json)) {
return;
}

const predicate: (s: string) => boolean = typeof(moduleOrPredicate) === 'string'
? x => x === moduleOrPredicate
: moduleOrPredicate;

for (const m of Object.keys(this.json.dependencies)) {
if (predicate(m)) {
delete this.json.dependencies[m];
}
}
}

public addPeerDependency(module: string, version: string) {
if (!('peerDependencies' in this.json)) {
this.json.peerDependencies = {};
Expand Down Expand Up @@ -250,6 +316,8 @@ export class PackageJson {
* Interface for validation rules
*/
export abstract class ValidationRule {
public abstract readonly name: string;

/**
* Will be executed for every package definition once, used to collect statistics
*/
Expand Down
Loading

0 comments on commit 7b3d8cd

Please sign in to comment.