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

* build: Switch @sentry/utils to using eslint

* build: Switch @sentry/apm to using eslint

* clean up other files

* feat: Use eslint cache

* build: Switch @sentry/angular to using eslint

* build: Switch @sentry/gatsby to using eslint

* build: Switch @sentry/ember to using eslint

* gitignore

* dangerfile

* add plugins for import

* cleanup

* fix

Author: AbhiPrasad

.eslintignore

@@ -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/**/*

.eslintrc.js

@@ -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: [
     {
@@ -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',
@@ -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',
       },
     },
     {
@@ -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',
       },
     },
     {
@@ -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' }],
   },
 };

.gitignore

@@ -34,3 +34,7 @@ lint-results.json
 
 # legacy
 tmp.js
+
+# eslint
+.eslintcache
+eslintcache/*

dangerfile.ts

@@ -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'];
 
 /**
@@ -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);

package.json

@@ -19,6 +19,7 @@
     "yarn": "1.13.0"
   },
   "workspaces": [
+    "packages/angular",
     "packages/apm",
     "packages/browser",
     "packages/core",
@@ -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",

packages/angular/.eslintrc.js

@@ -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',
+      },
+    },
+  ],
+};

packages/angular/package.json

@@ -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
 }

packages/angular/src/errorhandler.ts

@@ -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);
     }
 

packages/angular/src/tracing.ts

@@ -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';
@@ -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) {
@@ -80,7 +71,7 @@ export class TraceService {
       }
 
       if (activeTransaction) {
-        this.routingSpan = activeTransaction.startChild({
+        this._routingSpan = activeTransaction.startChild({
           description: `${navigationEvent.url}`,
           op: `angular.routing`,
           tags: {
@@ -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';
@@ -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`,
       });
@@ -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();
     }
   }
 }
@@ -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) {
@@ -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();
@@ -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();