Skip to content
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

tslint: use a better implementation of rule checking unused variables #2156

Merged
merged 4 commits into from
Jan 7, 2019
Merged
Show file tree
Hide file tree
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
63 changes: 63 additions & 0 deletions bin/sync-dev-deps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/usr/bin/env node
// Copyright IBM Corp. 2017,2018. All Rights Reserved.
// Node module: loopback-next
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

/**
* This is an internal script to synchronize versions of dev-dependencies
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos When should we use this command? My guess is when updating a tslint/typescript version in the root level package.json?
A nitpick: maybe add some explanation in the documentation to remind developers to run it when updating dependencies.

* from monorepo's package.json to individual example packages.
*/
'use strict';

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

const Project = require('@lerna/project');

async function syncDevDeps() {
const project = new Project(process.cwd());
const packages = await project.getPackages();

const rootPath = project.rootPath;

// Load dependencies from `packages/build/package.json`
const buildDeps = require(path.join(rootPath, 'packages/build/package.json'))
.dependencies;

const masterDeps = {
typescript: buildDeps.typescript,
tslint: buildDeps.tslint,
};

// Update typescript & tslint dependencies in individual packages
for (const pkg of packages) {
const data = readJsonFile(pkg.manifestLocation);
let modified = false;
for (const dep in masterDeps) {
if (data.devDependencies && dep in data.devDependencies) {
data.devDependencies[dep] = masterDeps[dep];
modified = true;
}
}
if (!modified) continue;
writeJsonFile(pkg.manifestLocation, data);
}

// Update dependencies in monorepo root
const rootPackage = path.join(rootPath, 'package.json');
const data = readJsonFile(rootPackage);
Object.assign(data.devDependencies, masterDeps);
writeJsonFile(rootPackage, data);
}

if (require.main === module) syncDevDeps();

function readJsonFile(filePath) {
return JSON.parse(fs.readFileSync(filePath, 'utf-8'));
}

function writeJsonFile(filePath, data) {
fs.writeFileSync(filePath, JSON.stringify(data, null, 2) + '\n', 'utf-8');
console.log('%s has been updated.', filePath);
}
2 changes: 1 addition & 1 deletion bin/travis.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ set -e
# Running Code Linter -- Requires @loopback/build so it's bootstrapped
if [ $TASK = "code-lint" ]; then
echo "TASK => LINTING CODE"
lerna bootstrap --scope @loopback/build
lerna bootstrap --scope @loopback/build --scope @loopback/tslint-config
npm run lint

# Commit Message Linter
Expand Down
22 changes: 22 additions & 0 deletions docs/site/DEVELOPING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ See [Monorepo overview](./MONOREPO.md) for a list of all packages.
- [Making breaking changes](#making-breaking-changes)
- [Releasing new versions](#releasing-new-versions)
- [Adding a new package](#adding-a-new-package)
- [Upgrading TypeScript/tslint](#upgrading-typescripttslint)
- [How to test infrastructure changes](#how-to-test-infrastructure-changes)

## Setting up development environment
Expand Down Expand Up @@ -437,6 +438,27 @@ Please register the new package in the following files:
[@raymondfeng](https://github.com/raymondfeng) to enlist the new package on
<http://apidocs.loopback.io/>.

## Upgrading TypeScript/tslint

In order to support tslint extensions with a peer dependency on tslint, we have
to specify `typescript` and `tslint` dependency in multiple places in our
monorepo.

Steps to upgrade `typescript` or `tslint` to a newer version:

1. Update the dependencies in `@loopback/build`, this is the source of truth for
the rest of the monorepo.

```shell
$ (cd packages/build && npm update typescript tslint)
```

2. Propagate the change to other places to keep everything consistent.

```shell
$ node bin/sync-dev-deps
```

## How to test infrastructure changes

When making changes to project infrastructure, e.g. modifying `tsc` or `tslint`
Expand Down
4 changes: 3 additions & 1 deletion examples/hello-world/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
"@loopback/build": "^1.1.0",
"@loopback/testlab": "^1.0.3",
"@loopback/tslint-config": "^1.0.0",
"@types/node": "^10.11.2"
"@types/node": "^10.11.2",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"keywords": [
"loopback",
Expand Down
4 changes: 3 additions & 1 deletion examples/log-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@
"@loopback/testlab": "^1.0.3",
"@loopback/tslint-config": "^1.0.0",
"@types/debug": "0.0.30",
"@types/node": "^10.11.2"
"@types/node": "^10.11.2",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"dependencies": {
"@loopback/context": "^1.4.0",
Expand Down
4 changes: 3 additions & 1 deletion examples/rpc-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
"@loopback/tslint-config": "^1.0.0",
"@types/express": "^4.11.1",
"@types/node": "^10.11.2",
"@types/p-event": "^1.3.0"
"@types/p-event": "^1.3.0",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
}
}
4 changes: 3 additions & 1 deletion examples/soap-calculator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
"@types/mocha": "^5.0.0",
"@types/node": "^10.11.2",
"mocha": "^5.1.1",
"source-map-support": "^0.5.5"
"source-map-support": "^0.5.5",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
}
}
4 changes: 3 additions & 1 deletion examples/todo-list/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@
"@loopback/tslint-config": "^1.0.0",
"@types/lodash": "^4.14.109",
"@types/node": "^10.11.2",
"lodash": "^4.17.10"
"lodash": "^4.17.10",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"keywords": [
"loopback",
Expand Down
4 changes: 3 additions & 1 deletion examples/todo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@
"@loopback/tslint-config": "^1.0.0",
"@types/lodash": "^4.14.109",
"@types/node": "^10.11.2",
"lodash": "^4.17.10"
"lodash": "^4.17.10",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"keywords": [
"loopback",
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
"coveralls": "^3.0.0",
"cz-conventional-changelog": "^2.1.0",
"husky": "^1.2.0",
"lerna": "^3.5.1"
"lerna": "^3.5.1",
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"scripts": {
"postinstall": "lerna bootstrap",
"prerelease": "npm run build:full && npm run mocha && npm run lint",
"release": "lerna version && lerna publish from-git --yes",
"update-template-deps": "node bin/update-template-deps -f",
"sync-dev-deps": "node bin/sync-dev-deps",
"version": "npm run update-template-deps && npm run apidocs",
"outdated": "npm outdated --depth 0 && lerna exec --no-bail \"npm outdated --depth 0\"",
"apidocs": "node bin/run-lerna run build:apidocs",
Expand Down
2 changes: 1 addition & 1 deletion packages/boot/test/fixtures/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {ServiceMixin} from '@loopback/service-proxy';
import {BootMixin} from '../..';

// Force package.json to be copied to `dist` by `tsc`
//tslint:disable-next-line:no-unused-variable
//tslint:disable-next-line:no-unused
import * as pkg from './package.json';

export class BooterApp extends BootMixin(
Expand Down
4 changes: 2 additions & 2 deletions packages/build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
"rimraf": "^2.6.2",
"source-map-support": "^0.5.5",
"strong-docs": "^4.0.0",
"tslint": "^5.9.1",
"typescript": "^3.2.1"
"tslint": "^5.12.0",
"typescript": "^3.2.2"
},
"bin": {
"lb-tsc": "./bin/compile-package.js",
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/generators/project/templates/package.json.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@
"@loopback/build": "<%= project.dependencies['@loopback/build'] -%>",
"@loopback/testlab": "<%= project.dependencies['@loopback/testlab'] -%>",
"@loopback/tslint-config": "<%= project.dependencies['@loopback/tslint-config'] -%>",
"@types/node": "<%= project.dependencies['@types/node'] -%>"
"@types/node": "<%= project.dependencies['@types/node'] -%>",
<% if (project.tslint) { -%>
"tslint": "<%= project.dependencies['tslint'] -%>",
<% } -%>
"typescript": "<%= project.dependencies['typescript'] -%>"
}
}
4 changes: 3 additions & 1 deletion packages/cli/test/integration/lib/project-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,12 @@ module.exports = function(projGenerator, props, projectType) {
assert.jsonFileContent('package.json', props);
assert.fileContent([
['package.json', '@loopback/build'],
['package.json', '"typescript"'],
['package.json', '"tslint"'],
['tslint.json', '@loopback/tslint-config'],
['tsconfig.json', '@loopback/build'],
]);
assert.noFileContent([
['package.json', '"typescript"'],
['tslint.json', '"rules"'],
['tsconfig.json', '"compilerOptions"'],
]);
Expand Down Expand Up @@ -423,6 +424,7 @@ module.exports = function(projGenerator, props, projectType) {
assert.noFile(['tslint.json', 'tslint.build.json']);
assert.noFileContent([
['package.json', '@loopback/build'],
['package.json', '"tslint"'],
['tsconfig.json', '@loopback/build'],
]);
assert.fileContent([
Expand Down
2 changes: 1 addition & 1 deletion packages/context/src/binding-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

export type BindingAddress<T> = string | BindingKey<T>;

// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
export class BindingKey<ValueType> {
static readonly PROPERTY_SEPARATOR = '#';

Expand Down
2 changes: 1 addition & 1 deletion packages/context/src/resolution-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const debugSession = debugModule('loopback:context:resolver:session');
const getTargetName = DecoratorFactory.getTargetName;

// NOTE(bajtos) The following import is required to satisfy TypeScript compiler
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
import {BindingKey} from './binding-key';

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/context/test/unit/inject.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {

describe('function argument injection', () => {
it('can decorate class constructor arguments', () => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class TestClass {
constructor(@inject('foo') foo: string) {}
}
Expand All @@ -29,7 +29,7 @@ describe('function argument injection', () => {
});

it('can retrieve information about injected method arguments', () => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class TestClass {
test(@inject('foo') foo: string) {}
}
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('function argument injection', () => {

describe('property injection', () => {
it('can decorate properties', () => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class TestClass {
@inject('foo')
foo: string;
Expand Down Expand Up @@ -126,7 +126,7 @@ describe('property injection', () => {

it('cannot decorate static properties', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class TestClass {
@inject('foo')
static foo: string;
Expand All @@ -136,7 +136,7 @@ describe('property injection', () => {

it('cannot decorate a method', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class TestClass {
@inject('bar')
foo() {}
Expand Down
2 changes: 1 addition & 1 deletion packages/context/test/unit/resolution-session.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {ResolutionSession, Binding, Injection, inject} from '../..';

describe('ResolutionSession', () => {
class MyController {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
constructor(@inject('b') private b: string) {}
}
function givenInjection(): Injection {
Expand Down
1 change: 1 addition & 0 deletions packages/metadata/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type DecoratorType =
* @typeparam T Type of the metadata value
* @typeparam D Type of the decorator
*/
// tslint:disable-next-line:no-unused
bajtos marked this conversation as resolved.
Show resolved Hide resolved
export class MetadataAccessor<T, D extends DecoratorType = DecoratorType> {
private constructor(public readonly key: string) {}

Expand Down
16 changes: 8 additions & 8 deletions packages/metadata/test/unit/decorator-factory.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('ClassDecoratorFactory', () => {
expect(() => {
@classDecorator({x: 1})
@classDecorator({y: 2})
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {}
}).to.throw(
/Decorator cannot be applied more than once on class MyController/,
Expand Down Expand Up @@ -352,7 +352,7 @@ describe('PropertyDecoratorFactory', () => {

it('throws if applied more than once on the same property', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
@propertyDecorator({x: 1})
@propertyDecorator({y: 2})
Expand Down Expand Up @@ -400,7 +400,7 @@ describe('PropertyDecoratorFactory for static properties', () => {

it('throws if applied more than once on the same static property', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
@propertyDecorator({x: 1})
@propertyDecorator({y: 2})
Expand Down Expand Up @@ -448,7 +448,7 @@ describe('MethodDecoratorFactory', () => {

it('throws if applied more than once on the same method', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
@methodDecorator({x: 1})
@methodDecorator({y: 2})
Expand Down Expand Up @@ -496,7 +496,7 @@ describe('MethodDecoratorFactory for static methods', () => {

it('throws if applied more than once on the same static method', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
@methodDecorator({x: 1})
@methodDecorator({y: 2})
Expand Down Expand Up @@ -545,7 +545,7 @@ describe('ParameterDecoratorFactory', () => {

it('throws if applied more than once on the same parameter', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
myMethod(
@parameterDecorator({x: 1})
Expand Down Expand Up @@ -634,7 +634,7 @@ describe('ParameterDecoratorFactory for a static method', () => {

it('throws if applied more than once on the same parameter', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
static myMethod(
@parameterDecorator({x: 1})
Expand Down Expand Up @@ -695,7 +695,7 @@ describe('MethodParameterDecoratorFactory with invalid decorations', () => {

it('reports error if the # of decorations exceeeds the # of params', () => {
expect(() => {
// tslint:disable-next-line:no-unused-variable
// tslint:disable-next-line:no-unused
class MyController {
@methodParameterDecorator({x: 1}) // Causing error
@methodParameterDecorator({x: 2}) // For a
Expand Down
Loading