Skip to content

Commit

Permalink
BREAKING CHANGE: chore: sync with internal lint (#75)
Browse files Browse the repository at this point in the history
This synchronizes the lint rules to be closer to the lint rules used
internally at Google. Not all rules are available externally, so this
is not *exactly* the same.

Also updated the TypeScript version to 2.6.1.

A fair number of new lint rules are being applied now, which means that
the code will be alot more consistent with other Google code, but it
also means that users of gts may need to modify the source to pick up
this release. For most of the new lint rules, a fixer is also available
which should help with migration.

TypeScript 2.6.x enables contravariant types for function parameters,
which may also require some changes. See [1] for more info.

[1] https://blogs.msdn.microsoft.com/typescript/2017/10/31/announcing-typescript-2-6/
  • Loading branch information
ofrobots committed Nov 7, 2017
1 parent 7b3db0f commit dae4e41
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 54 deletions.
48 changes: 22 additions & 26 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
"compile": "tsc -p .",
"format-check": "./bin/format-check.sh",
"format": "clang-format -i -style='{Language: JavaScript, BasedOnStyle: Google, ColumnLimit: 80}' src/*.ts test/*.ts",
"lint": "tslint -c tslint.json --project . --type-check -t codeFrame",
"lint-fix": "tslint -c tslint.json --project . --type-check -t codeFrame --fix",
"lint": "tslint -c tslint.json --project . -t codeFrame",
"lint-fix": "tslint -c tslint.json --project . -t codeFrame --fix",
"prepare": "npm run compile",
"test": "npm run compile && nyc ava build/test/test*.js",
"posttest": "npm run lint && npm run format-check"
Expand All @@ -44,7 +44,7 @@
"meow": "^3.7.0",
"pify": "^3.0.0",
"rimraf": "^2.6.1",
"tslint": "^5.5.0",
"tslint": "^5.8.0",
"update-notifier": "^2.2.0",
"write-file-atomic": "^2.1.0"
},
Expand All @@ -69,7 +69,7 @@
"typescript": "~2.6.1"
},
"peerDependencies": {
"typescript": "^2.4.1"
"typescript": "^2.6.1"
},
"ava": {
"require": [
Expand Down
7 changes: 6 additions & 1 deletion src/clean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@
* limitations under the License.
*/
import chalk from 'chalk';
import * as ts from 'typescript';

import {Options} from './cli';
import {getTSConfig, rimrafp} from './util';

interface TSConfig {
compilerOptions: ts.CompilerOptions;
}

/**
* Remove files generated by the build.
*/
export async function clean(options: Options): Promise<boolean> {
const tsconfig = (await getTSConfig(options.targetRootDir));
const tsconfig = (await getTSConfig(options.targetRootDir)) as TSConfig;
if (tsconfig.compilerOptions && tsconfig.compilerOptions.outDir) {
const outDir = tsconfig.compilerOptions.outDir;
if (outDir === '.') {
Expand Down
9 changes: 5 additions & 4 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import {init} from './init';
import {clean} from './clean';

export interface Logger {
log: (...args: any[]) => void;
error: (...args: any[]) => void;
dir: (obj: any, options?: any) => void;
log: (...args: Array<{}>) => void;
error: (...args: Array<{}>) => void;
dir: (obj: {}, options?: {}) => void;
}

export interface Options {
Expand Down Expand Up @@ -74,14 +74,15 @@ async function run(verb: string, files: string[]): Promise<boolean> {
gtsRootDir: `${process.cwd()}/node_modules/gts`,
targetRootDir: process.cwd(),
yes: cli.flags.yes || cli.flags.y || false,
logger: logger
logger
};
// Linting/formatting depend on typescript. We don't want to load the
// typescript module during init, since it might not exist.
// See: https://github.com/google/ts-style/issues/48
if (verb === 'init') {
return await init(options);
}

const lint: VerbFilesFunction = require('./lint').lint;
const format: VerbFilesFunction = require('./format').format;
switch (verb) {
Expand Down
22 changes: 15 additions & 7 deletions src/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ import * as path from 'path';
import {Options} from './cli';
import {readFilep as read, readJsonp as readJson, writeFileAtomicp as write} from './util';

interface Bag<T> {
[script: string]: T;
const pkg = require('../../package.json') as PackageJson;

export interface Bag<T> { [script: string]: T; }

// TODO: is this type available from definitelytyped.org? Find it, and drop the
// local definition.
export interface PackageJson {
version?: string;
devDependencies?: Bag<string>;
scripts?: Bag<string>;
}

async function query(
Expand All @@ -42,7 +50,7 @@ async function query(
}

async function addScripts(
packageJson: any, options: Options): Promise<boolean> {
packageJson: PackageJson, options: Options): Promise<boolean> {
let edits = false;
const scripts: Bag<string> = {
check: `gts check`,
Expand Down Expand Up @@ -81,9 +89,9 @@ async function addScripts(
}

async function addDependencies(
packageJson: any, options: Options): Promise<boolean> {
packageJson: PackageJson, options: Options): Promise<boolean> {
let edits = false;
const deps: Bag<string> = {'gts': 'latest', 'typescript': '^2.4.1'};
const deps: Bag<string> = {'gts': `^${pkg.version}`, 'typescript': '^2.6.1'};

if (!packageJson.devDependencies) {
packageJson.devDependencies = {};
Expand Down Expand Up @@ -111,14 +119,14 @@ async function addDependencies(
return edits;
}

function formatJson(object: any) {
function formatJson(object: {}) {
// TODO: preserve the indent from the input file.
const json = JSON.stringify(object, null, ' ');
return `${json}\n`;
}

async function writePackageJson(
packageJson: any, options: Options): Promise<void> {
packageJson: PackageJson, options: Options): Promise<void> {
options.logger.log('Writing package.json...');
if (!options.dryRun) {
await write('./package.json', formatJson(packageJson));
Expand Down
2 changes: 1 addition & 1 deletion src/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function lint(
const program = createProgram(options);
const configuration =
Configuration.findConfiguration(tslintConfigPath, '').results;
const linter = new Linter({fix: fix, formatter: 'codeFrame'}, program);
const linter = new Linter({fix, formatter: 'codeFrame'}, program);
const srcFiles = files.length > 0 ? files : Linter.getFileNames(program);
srcFiles.forEach(file => {
const fileContents = program.getSourceFile(file).getFullText();
Expand Down
6 changes: 4 additions & 2 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export async function readJsonp(jsonPath: string) {
return JSON.parse(await readFilep(jsonPath));
}

export interface ReadFileP { (path: string, encoding: string): Promise<any>; }
export interface ReadFileP {
(path: string, encoding: string): Promise<string>;
}

export function nop() {
/* empty */
Expand All @@ -38,7 +40,7 @@ export function nop() {
* @param rootDir Directory where the tsconfig.json should be found.
*/
export async function getTSConfig(
rootDir: string, customReadFilep?: ReadFileP): Promise<any> {
rootDir: string, customReadFilep?: ReadFileP): Promise<{}> {
const tsconfigPath = path.join(rootDir, 'tsconfig.json');
customReadFilep = customReadFilep || readFilep;
const json = await customReadFilep(tsconfigPath, 'utf8');
Expand Down
6 changes: 3 additions & 3 deletions test/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export interface Fixtures {
async function setupFixtures(dir: string, fixtures: Fixtures) {
await makeDir(dir);
const keys = Object.keys(fixtures);
for (let key of keys) {
for (const key of keys) {
const filePath = path.join(dir, key);
if (typeof fixtures[key] === 'string') {
const contents = fixtures[key] as string;
Expand All @@ -46,9 +46,9 @@ async function setupFixtures(dir: string, fixtures: Fixtures) {
}

export async function withFixtures(
fixtures: Fixtures, fn: (fixturesDir: string) => Promise<any>) {
fixtures: Fixtures, fn: (fixturesDir: string) => Promise<{}|void>) {
const keep = !!process.env.GTS_KEEP_TEMPDIRS;
const dir = tmp.dirSync({keep: keep, unsafeCleanup: true});
const dir = tmp.dirSync({keep, unsafeCleanup: true});

await setupFixtures(dir.name, fixtures);

Expand Down
4 changes: 2 additions & 2 deletions test/test-kitchen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface ExecError extends Error {
}

function isExecError(err: Error|ExecError): err is ExecError {
return (<ExecError>err).code !== undefined;
return (err as ExecError).code !== undefined;
}

// cp.exec doesn't fit the (err ^ result) pattern because a process can write
Expand All @@ -54,7 +54,7 @@ const execp =
};

const keep = !!process.env.GTS_KEEP_TEMPDIRS;
const stagingDir = tmp.dirSync({keep: keep, unsafeCleanup: true});
const stagingDir = tmp.dirSync({keep, unsafeCleanup: true});
const stagingPath = stagingDir.name;
const execOpts = {
cwd: `${stagingPath}/kitchen`
Expand Down
3 changes: 2 additions & 1 deletion test/test-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {getTSConfig} from '../src/util';
test('get should parse the correct tsconfig file', async t => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG = {a: 'b'};
function fakeReadFilep(configPath: string, encoding: string): Promise<any> {
function fakeReadFilep(
configPath: string, encoding: string): Promise<string> {
t.is(configPath, path.join(FAKE_DIRECTORY, 'tsconfig.json'));
t.is(encoding, 'utf8');
return Promise.resolve(JSON.stringify(FAKE_CONFIG));
Expand Down
39 changes: 36 additions & 3 deletions tslint.json
Original file line number Diff line number Diff line change
@@ -1,31 +1,64 @@
{
"rules": {
"array-type": [true, "array-simple"],
"arrow-return-shorthand": true,
"ban": [true,
{"name": "parseInt", "message": "tsstyle#type-coercion"},
{"name": "parseFloat", "message": "tsstyle#type-coercion"},
{"name": "Array", "message": "tsstyle#array-constructor"}
],
"ban-types": [true,
["Object", "Use {} instead."],
["String", "Use 'string' instead."],
["Number", "Use 'number' instead."],
["Boolean", "Use 'boolean' instead."]
],
"class-name": true,
"comment-format": [true, "check-space"],
"curly": [true, "ignore-same-line"],
"eofline": true,
"forin": true,
"indent": [true, "spaces"],
"interface-name": [true, "never-prefix"],
"jsdoc-format": true,
"label-position": true,
"member-access": [true, "no-public"],
"new-parens": true,
"no-angle-bracket-type-assertion": true,
"no-any": true,
"no-arg": true,
"no-conditional-assignment": true,
"no-construct": true,
"no-debugger": true,
"no-default-export": true,
"no-duplicate-variable": true,
"no-empty": true,
"no-inferrable-types": true,
"no-internal-module": true,
"no-namespace": [true, "allow-declarations"],
"no-reference": true,
"no-shadowed-variable": true,
"no-string-throw": true,
"no-switch-case-fall-through": true,
"no-trailing-whitespace": true,
"no-unused-expression": true,
"no-use-before-declare": true,
"no-var-keyword": true,
"object-literal-shorthand": true,
"only-arrow-functions": [true, "allow-declarations", "allow-named-functions"],
"prefer-const": true,
"quotemark": [true, "single"],
"radix": true,
"semicolon": [true, "always"],
"semicolon": [true, "always", "ignore-bound-class-methods"],
"switch-default": true,
"triple-equals": [true, "allow-null-check"],
"variable-name": [true, "check-format", "ban-keywords"]
"use-isnan": true,
"variable-name": [
true,
"check-format",
"ban-keywords",
"allow-leading-underscore",
"allow-trailing-underscore"
]
}
}
}

0 comments on commit dae4e41

Please sign in to comment.