Skip to content

build: Convert utils, apm, angular, gatsby, ember to eslint #2804

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

Merged
merged 12 commits into from
Aug 11, 2020
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
4 changes: 0 additions & 4 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
# THIS IS A TEMPORARY FILE
# THIS WILL BE REMOVED AFTER WE FINISH ESLINT UPGRADE

packages/apm/**/*
packages/ember/**/*
packages/gatsby/**/*
packages/integrations/**/*
packages/node/**/*
packages/react/**/*
packages/tracing/**/*
packages/typescript/**/*
packages/utils/**/*
44 changes: 40 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ module.exports = {
env: {
node: true,
},
extends: ['prettier', 'eslint:recommended'],
plugins: ['sentry-sdk', 'jsdoc'],
extends: ['prettier', 'eslint:recommended', 'plugin:import/errors', 'plugin:import/warnings'],
plugins: ['sentry-sdk', 'simple-import-sort'],
ignorePatterns: ['eslint-plugin-sentry-sdk'],
overrides: [
{
Expand All @@ -17,8 +17,8 @@ module.exports = {
{
// Configuration for typescript files
files: ['*.ts', '*.tsx', '*.d.ts'],
extends: ['plugin:@typescript-eslint/recommended', 'prettier/@typescript-eslint'],
plugins: ['@typescript-eslint'],
extends: ['plugin:@typescript-eslint/recommended', 'prettier/@typescript-eslint', 'plugin:import/typescript'],
plugins: ['@typescript-eslint', 'jsdoc', 'deprecation'],
parser: '@typescript-eslint/parser',
parserOptions: {
project: './tsconfig.json',
Expand Down Expand Up @@ -68,6 +68,25 @@ module.exports = {
leadingUnderscore: 'require',
},
],

// Prefer for-of loop over for loop if index is only used to access array
'@typescript-eslint/prefer-for-of': 'error',

// Make sure all expressions are used. Turned off in tests
// Must disable base rule to prevent false positives
'no-unused-expressions': 'off',
'@typescript-eslint/no-unused-expressions': 'error',

// Make sure Promises are handled appropriately
'@typescript-eslint/no-floating-promises': 'error',

// Do not use deprecated methods
'deprecation/deprecation': 'error',

// sort imports
'simple-import-sort/sort': 'error',
'sort-imports': 'off',
'import/order': 'off',
},
},
{
Expand Down Expand Up @@ -95,6 +114,8 @@ module.exports = {
'max-lines': 'off',

'@typescript-eslint/explicit-function-return-type': 'off',
'no-unused-expressions': 'off',
'@typescript-eslint/no-unused-expressions': 'off',
},
},
{
Expand Down Expand Up @@ -128,5 +149,20 @@ module.exports = {

// We should require a whitespace beginning a comment
'spaced-comment': 'error',

// Disallow usage of bitwise operators - this makes it an opt in operation
'no-bitwise': 'error',

// Limit cyclomatic complexity
complexity: 'error',

// Make sure all expressions are used. Turn off on tests.
'no-unused-expressions': 'error',

// We shouldn't make assumptions about imports/exports being dereferenced.
'import/namespace': 'off',

// imports should be ordered.
'import/order': ['error', { 'newlines-between': 'always' }],
},
};
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ lint-results.json

# legacy
tmp.js

# eslint
.eslintcache
eslintcache/*
8 changes: 5 additions & 3 deletions dangerfile.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { exec } from 'child_process';
import { danger, fail, message, schedule, warn } from 'danger';
import { promisify } from 'util';
import { resolve } from 'path';
import tslint from 'danger-plugin-tslint';
import { prettyResults } from 'danger-plugin-tslint/dist/prettyResults';
import { CLIEngine } from 'eslint';
import { resolve } from 'path';
import { promisify } from 'util';

const PACKAGES = ['apm', 'integrations', 'node', 'utils'];
const PACKAGES = ['integrations', 'node'];
const EXTENSIONS = ['.js', '.jsx', '.ts', '.tsx'];

/**
Expand All @@ -15,13 +15,15 @@ const EXTENSIONS = ['.js', '.jsx', '.ts', '.tsx'];
*/
async function eslint(): Promise<void[]> {
const allFiles = danger.git.created_files.concat(danger.git.modified_files);
// eslint-disable-next-line deprecation/deprecation
const cli = new CLIEngine({});
// let eslint filter down to non-ignored, matching the extensions expected
const filesToLint = allFiles.filter(f => !cli.isPathIgnored(f) && EXTENSIONS.some(ext => f.endsWith(ext)));
return Promise.all(filesToLint.map(f => lintFile(cli, f)));
}

/** JSDoc */
// eslint-disable-next-line deprecation/deprecation
async function lintFile(linter: CLIEngine, path: string): Promise<void> {
const contents = await danger.github.utils.fileContents(path);
const report = linter.executeOnText(contents, path);
Expand Down
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"yarn": "1.13.0"
},
"workspaces": [
"packages/angular",
"packages/apm",
"packages/browser",
"packages/core",
Expand Down Expand Up @@ -52,8 +53,11 @@
"danger-plugin-tslint": "^2.0.0",
"eslint": "^7.5.0",
"eslint-config-prettier": "^6.11.0",
"eslint-plugin-deprecation": "^1.1.0",
"eslint-plugin-import": "^2.22.0",
"eslint-plugin-jsdoc": "^30.0.3",
"eslint-plugin-sentry-sdk": "file:./eslint-plugin-sentry-sdk",
"eslint-plugin-simple-import-sort": "^5.0.3",
"jest": "^24.7.1",
"karma-browserstack-launcher": "^1.5.1",
"karma-firefox-launcher": "^1.1.0",
Expand Down
20 changes: 20 additions & 0 deletions packages/angular/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module.exports = {
root: true,
env: {
es6: true,
browser: true,
},
parserOptions: {
ecmaVersion: 2018,
},
extends: ['../../.eslintrc.js'],
ignorePatterns: ['build/**/*', 'dist/**/*', 'esm/**/*', 'examples/**/*', 'scripts/**/*'],
overrides: [
{
files: ['*.ts', '*.tsx', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
],
};
13 changes: 6 additions & 7 deletions packages/angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@
"build:watch:esm": "tsc -p tsconfig.esm.json -w --preserveWatchOutput",
"clean": "rimraf dist coverage build esm",
"link:yarn": "yarn link",
"lint": "run-s lint:prettier lint:tslint",
"lint:prettier": "prettier-check \"{src,test}/**/*.{ts,tsx}\"",
"lint:tslint": "tslint -t stylish -p .",
"lint:tslint:json": "tslint --format json -p . | tee lint-results.json",
"fix": "run-s fix:tslint fix:prettier",
"fix:prettier": "prettier --write \"{src,test}/**/*.{ts,tsx}\"",
"fix:tslint": "tslint --fix -t stylish -p ."
"lint": "run-s lint:prettier lint:eslint",
"lint:prettier": "prettier-check \"{src,test}/**/*.ts\"",
"lint:eslint": "eslint . --cache --cache-location '../../eslintcache/' --format stylish",
"fix": "run-s fix:eslint fix:prettier",
"fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
"fix:eslint": "eslint . --format stylish --fix"
},
"sideEffects": false
}
1 change: 1 addition & 0 deletions packages/angular/src/errorhandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class SentryErrorHandler implements AngularErrorHandler {

// When in development mode, log the error to console for immediate feedback.
if (this._options.logErrors) {
// eslint-disable-next-line no-console
console.error(extractedError);
}

Expand Down
49 changes: 24 additions & 25 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// tslint:disable:max-classes-per-file

import { AfterViewInit, Directive, Injectable, Input, OnInit } from '@angular/core';
import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
import { getCurrentHub } from '@sentry/browser';
Expand Down Expand Up @@ -54,14 +52,7 @@ export function getActiveTransaction(): Transaction | undefined {
*/
@Injectable({ providedIn: 'root' })
export class TraceService {
private routingSpan?: Span;

public constructor(private readonly router: Router) {
this.navStart$.subscribe();
this.navEnd$.subscribe();
}

public navStart$: Observable<Event> = this.router.events.pipe(
public navStart$: Observable<Event> = this._router.events.pipe(
filter(event => event instanceof NavigationStart),
tap(event => {
if (!instrumentationInitialized) {
Expand All @@ -80,7 +71,7 @@ export class TraceService {
}

if (activeTransaction) {
this.routingSpan = activeTransaction.startChild({
this._routingSpan = activeTransaction.startChild({
description: `${navigationEvent.url}`,
op: `angular.routing`,
tags: {
Expand All @@ -95,15 +86,22 @@ export class TraceService {
}),
);

public navEnd$: Observable<Event> = this.router.events.pipe(
public navEnd$: Observable<Event> = this._router.events.pipe(
filter(event => event instanceof NavigationEnd),
tap(() => {
if (this.routingSpan) {
this.routingSpan.finish();
delete this.routingSpan;
if (this._routingSpan) {
this._routingSpan.finish();
delete this._routingSpan;
}
}),
);

private _routingSpan?: Span;

public constructor(private readonly _router: Router) {
this.navStart$.subscribe();
this.navEnd$.subscribe();
}
}

const UNKNOWN_COMPONENT = 'unknown';
Expand All @@ -113,18 +111,18 @@ const UNKNOWN_COMPONENT = 'unknown';
*/
@Directive({ selector: '[trace]' })
export class TraceDirective implements OnInit, AfterViewInit {
private tracingSpan?: Span;

@Input('trace') public componentName: string = UNKNOWN_COMPONENT;

private _tracingSpan?: Span;

/**
* Implementation of OnInit lifecycle method
* @inheritdoc
*/
public ngOnInit(): void {
const activeTransaction = getActiveTransaction();
if (activeTransaction) {
this.tracingSpan = activeTransaction.startChild({
this._tracingSpan = activeTransaction.startChild({
description: `<${this.componentName}>`,
op: `angular.initialize`,
});
Expand All @@ -136,8 +134,8 @@ export class TraceDirective implements OnInit, AfterViewInit {
* @inheritdoc
*/
public ngAfterViewInit(): void {
if (this.tracingSpan) {
this.tracingSpan.finish();
if (this._tracingSpan) {
this._tracingSpan.finish();
}
}
}
Expand All @@ -148,10 +146,11 @@ export class TraceDirective implements OnInit, AfterViewInit {
export function TraceClassDecorator(): ClassDecorator {
let tracingSpan: Span;

return (target: Function) => {
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
return target => {
// tslint:disable-next-line:no-unsafe-any
const originalOnInit = target.prototype.ngOnInit;
// tslint:disable-next-line:no-unsafe-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any
target.prototype.ngOnInit = function(...args: any[]): ReturnType<typeof originalOnInit> {
const activeTransaction = getActiveTransaction();
if (activeTransaction) {
Expand All @@ -161,14 +160,12 @@ export function TraceClassDecorator(): ClassDecorator {
});
}
if (originalOnInit) {
// tslint:disable-next-line:no-unsafe-any
return originalOnInit.apply(this, args);
}
};

// tslint:disable-next-line:no-unsafe-any
const originalAfterViewInit = target.prototype.ngAfterViewInit;
// tslint:disable-next-line:no-unsafe-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any
target.prototype.ngAfterViewInit = function(...args: any[]): ReturnType<typeof originalAfterViewInit> {
if (tracingSpan) {
tracingSpan.finish();
Expand All @@ -185,8 +182,10 @@ export function TraceClassDecorator(): ClassDecorator {
* Decorator function that can be used to capture a single lifecycle methods of the component.
*/
export function TraceMethodDecorator(): MethodDecorator {
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/ban-types
return (target: Object, propertyKey: string | symbol, descriptor: PropertyDescriptor) => {
const originalMethod = descriptor.value;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
descriptor.value = function(...args: any[]): ReturnType<typeof originalMethod> {
const now = timestampWithMs();
const activeTransaction = getActiveTransaction();
Expand Down
3 changes: 0 additions & 3 deletions packages/angular/tslint.json

This file was deleted.

Loading