Merge pull request #170 from NullVoxPopuli/fix-typescript-prettier-co…

…mpat

Compat with TypeScript's declare syntax (+ prettier)

lib/rules/decorator-position.js

@@ -88,8 +88,12 @@ module.exports = {
 // ///////////////////////////////////////////////////////////
 // Everything below here can be copied into astexplorer.net
 // parser: babel-eslint
+//      or @typecsript-eslint/parser
 // transform: ESLint v4
 //
+// uncomment this line:
+// module.exports = decoratorPositionRule
+//
 // example config (context.options[0]):
 //
 // NOTE: implementing this will need a breaking change
@@ -135,8 +139,8 @@ function decoratorPositionRule(context) {
 
   const userOptions = context.options[0] || {};
   const filePath = context.getFilename();
-  const prettierOptions = lineLength(userOptions, filePath);
-  const options = normalizeOptions(prettierOptions);
+  const { printWidth } = lineLength(userOptions, filePath);
+  const options = normalizeOptions({ ...userOptions, printWidth });
 
   return {
     'ClassProperty[decorators.length=1]:exit'(node) {
@@ -200,21 +204,16 @@ function positionDecorator(context, node, options) {
 }
 
 function placeDecoratorsBesideProperty(context, node, options) {
+  const printWidth = Number(options.printWidth);
+
   for (const decoratorConfig of options.overrides[PREFER_INLINE]) {
     if (!decoratorConfig) {
       continue;
     }
-    const config = normalizeConfig(decoratorConfig, INTENT.SAME_LINE);
-    const decorator = node.decorators[0];
-    const token = context.getSourceCode().getTokenAfter(decorator, { includeComments: true });
-
-    const whitespaceStart = decorator.range[1];
-    const whitespaceEnd = token.range[0];
 
-    const whitespaceLength = whitespaceEnd - whitespaceStart;
-
-    const totalLineLength = calculateTotalLineLength(context, node, token) + whitespaceLength;
-    const lessThanOrEqualToPrintWidth = totalLineLength <= Number(options.printWidth);
+    const decorator = node.decorators[0];
+    const totalLineLength = lengthAsInline(context, node);
+    const lessThanOrEqualToPrintWidth = totalLineLength <= printWidth;
 
     if (!lessThanOrEqualToPrintWidth) {
       const forwardOptions = {
@@ -224,7 +223,8 @@ function placeDecoratorsBesideProperty(context, node, options) {
       placeDecoratorsAboveProperty(context, node, forwardOptions);
     }
 
-    const info = decoratorInfo(node, config);
+    const config = normalizeConfig(decoratorConfig, INTENT.SAME_LINE);
+    const info = decoratorInfo(context, node, config, options);
 
     if (!info.needsTransform) {
       continue;
@@ -255,7 +255,7 @@ function placeDecoratorsAboveProperty(context, node, options) {
       continue;
     }
     const config = normalizeConfig(decoratorConfig, INTENT.DIFFERENT_LINES);
-    const info = decoratorInfo(node, config);
+    const info = decoratorInfo(context, node, config, options);
 
     const decorator = node.decorators[0];
 
@@ -288,17 +288,6 @@ function placeDecoratorsAboveProperty(context, node, options) {
 // Helpers
 // ///////////////////////////////////
 
-function calculateTotalLineLength(context, node, token) {
-  const decorator = node.decorators[0];
-  const punctuator = context.getSourceCode().getTokenAfter(token, { includeComments: true });
-
-  const [decStart, decEnd] = decorator.range;
-  const [tokenStart, tokenEnd] = token.range;
-  const [puncStart, puncEnd] = punctuator.range;
-
-  return decEnd - decStart + tokenEnd - tokenStart + puncEnd - puncStart;
-}
-
 function normalizeOptions(userOptions) {
   const options = Object.assign({}, defaultOptions, userOptions);
 
@@ -371,8 +360,20 @@ function linePositioning(decorator, key) {
   return { onDifferentLines, onSameLine, isMultiLineDecorator };
 }
 
-function decoratorInfo(node, config) {
-  const [name, options] = config;
+function lengthAsInline(context, node) {
+  // Includes:
+  // - decorator(s)
+  // - declare
+  // - property name
+  // - type annotation (and !)
+  // - etc
+  return context.getSourceCode().getText(node).replace(/\s+/, ' ').length;
+}
+
+function decoratorInfo(context, node, decoratorConfig, options) {
+  const printWidth = Number(options.printWidth);
+
+  const [name, decoratorOptions] = decoratorConfig;
   const { decorators, key } = node;
   const decorator = decorators.find((decorator) => {
     return nameOfDecorator(decorator) === name;
@@ -382,24 +383,35 @@ function decoratorInfo(node, config) {
     return {};
   }
 
+  const inlineLength = lengthAsInline(context, node);
+  const ifInlineWouldViolatePrettier = inlineLength > printWidth;
+
   const decoratorName = nameOfDecorator(decorator);
   const arity = arityOfDecorator(decorator);
   const arityMatches =
     // we don't care what the args are, if they exist
-    options.withArgs === undefined ||
+    decoratorOptions.withArgs === undefined ||
     // this config requires args, so ensure the decorator has them
-    (options.withArgs === true && arity >= 0) ||
+    (decoratorOptions.withArgs === true && arity >= 0) ||
     // this config requires no args, so ensure the decorator doesn't have them
-    (options.withArgs === false && arity === undefined);
+    (decoratorOptions.withArgs === false && arity === undefined);
 
   const positioning = linePositioning(decorator, key);
   const currentPositionMatchesIntent =
-    (options.intent === INTENT.SAME_LINE && positioning.onSameLine) ||
-    (options.intent === INTENT.DIFFERENT_LINES && positioning.onDifferentLines);
+    (decoratorOptions.intent === INTENT.SAME_LINE && positioning.onSameLine) ||
+    (decoratorOptions.intent === INTENT.DIFFERENT_LINES && positioning.onDifferentLines);
 
   let needsTransform = arityMatches && Boolean(decorator && !currentPositionMatchesIntent);
 
-  if (options.intent === INTENT.SAME_LINE && positioning.isMultiLineDecorator) {
+  if (decoratorOptions.intent === INTENT.SAME_LINE && positioning.isMultiLineDecorator) {
+    needsTransform = false;
+  }
+
+  if (
+    decoratorOptions === INTENT.SAME_LINE &&
+    positioning.onDifferentLines &&
+    ifInlineWouldViolatePrettier
+  ) {
     needsTransform = false;
   }
 
@@ -409,6 +421,8 @@ function decoratorInfo(node, config) {
     arityMatches,
     currentPositionMatchesIntent,
     needsTransform,
+    inlineLength,
+    ifInlineWouldViolatePrettier,
     name: decoratorName,
     ...positioning,
   };

package.json

@@ -101,7 +101,8 @@
   },
   "jest": {
     "testPathIgnorePatterns": [
-      "<rootDir>/tests/helpers/"
+      "<rootDir>/tests/helpers/",
+      "<rootDir>/smoke-tests/"
     ],
     "testMatch": [
       "**/tests/**/*.js"

smoke-tests/integration/external-config-prettier/package.json

@@ -5,6 +5,8 @@
   "license": "MIT",
   "private": true,
   "dependencies": {
+    "@typescript-eslint/eslint-plugin": "*",
+    "@typescript-eslint/parser": "*",
     "babel-eslint": "*",
     "eslint-plugin-decorator-position": "*",
     "eslint": "*",

smoke-tests/integration/position-prettier/.eslintrc.js

@@ -14,4 +14,11 @@ module.exports = {
       },
     ],
   },
+  overrides: [
+    {
+      files: ['**/*.ts', '*.ts'],
+      parser: '@typescript-eslint/parser',
+      plugins: ['@typescript-eslint'],
+    },
+  ],
 };

smoke-tests/issue-reproductions/195/.eslintrc.js

@@ -0,0 +1,33 @@
+module.exports = {
+  env: {
+    es2021: true,
+  },
+  plugins: ['@typescript-eslint', 'prettier', 'decorator-position'],
+  parser: '@typescript-eslint/parser',
+  extends: [
+    'plugin:@typescript-eslint/recommended',
+    'plugin:decorator-position/ember',
+    'prettier',
+    'prettier/@typescript-eslint',
+  ],
+  parserOptions: {
+    ecmaFeatures: {
+      jsx: true,
+    },
+    ecmaVersion: 2018,
+    sourceType: 'module',
+  },
+  rules: {
+    '@typescript-eslint/no-explicit-any': ['error'],
+    'prettier/prettier': [
+      'error',
+      {
+        singleQuote: true,
+        printWidth: 100,
+        semi: true,
+        trailingComma: 'es5',
+        quoteProps: 'preserve',
+      },
+    ],
+  },
+};

smoke-tests/issue-reproductions/195/app.ts

@@ -0,0 +1,13 @@
+type Service = unknown;
+
+function service(name) {
+  return function decorator(target: unknown, key: string, descriptor?: PropertyDescriptor): void {
+    console.log(target, key, descriptor);
+  };
+}
+export default class Foo {
+  @service('addon-name/-private/do-not-use/the-name-of-the-service')
+  declare someObfuscatedPrivateService: Service;
+
+  @service('addon-name/-private/do-not-use/the-name-of-the-service') declare shortName: Service;
+}

smoke-tests/issue-reproductions/195/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "test",
+  "version": "0.0.0",
+  "description": "smoke-test",
+  "license": "MIT",
+  "private": true,
+  "dependencies": {
+    "@typescript-eslint/parser": "*",
+    "@typescript-eslint/eslint-plugin": "*",
+    "eslint-plugin-decorator-position": "*",
+    "eslint": "*",
+    "eslint-plugin-prettier": "*",
+    "eslint-config-prettier": "*"
+  }
+}

tests/lib/rules/decorator-position.js → tests/lib/rules/decorator-position-js.js

@@ -3,7 +3,6 @@
 //------------------------------------------------------------------------------
 
 const parser = require.resolve('babel-eslint');
-const tsParser = require.resolve('@typescript-eslint/parser');
 
 const { stripIndent } = require('common-tags');
 const RuleTester = require('eslint').RuleTester;
@@ -16,7 +15,6 @@ const rule = require('../../../lib/rules/decorator-position');
 //------------------------------------------------------------------------------
 
 const ruleTester = new RuleTester({ parser });
-const tsRuleTester = new RuleTester({ parser: tsParser });
 
 ruleTester.run('JS: decorator-position', rule, {
   valid: [
@@ -321,33 +319,3 @@ ruleTester.run('JS: decorator-position', rule, {
     },
   ],
 });
-
-tsRuleTester.run('TS: decorator-position', rule, {
-  valid: [
-    {
-      code: stripIndent`
-        export default class LocaleSwitcher extends Component<IArgs> {
-          @service locale!: LocaleService;
-        }
-      `,
-      options: [{ overrides: { 'prefer-inline': ['@service'] } }],
-    },
-  ],
-  invalid: [
-    {
-      code: stripIndent`
-        export default class LocaleSwitcher extends Component<IArgs> {
-          @service
-          locale!: LocaleService;
-        }
-      `,
-      options: [{ overrides: { 'prefer-inline': ['@service'] } }],
-      errors: [{ message: 'Expected @service to be inline.' }],
-      output: stripIndent`
-        export default class LocaleSwitcher extends Component<IArgs> {
-          @service locale!: LocaleService;
-        }
-      `,
-    },
-  ],
-});

tests/lib/rules/decorator-position-ts.js

@@ -0,0 +1,64 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const tsParser = require.resolve('@typescript-eslint/parser');
+
+const { stripIndent } = require('common-tags');
+const RuleTester = require('eslint').RuleTester;
+
+const rule = require('../../../lib/rules/decorator-position');
+// const { ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const tsRuleTester = new RuleTester({ parser: tsParser });
+
+tsRuleTester.run('TS: decorator-position', rule, {
+  valid: [
+    {
+      code: stripIndent`
+        export default class LocaleSwitcher extends Component<IArgs> {
+          @service locale!: LocaleService;
+        }
+      `,
+      options: [{ overrides: { 'prefer-inline': ['@service'] } }],
+    },
+    {
+      code: stripIndent`
+        export default class Foo {
+          // Would be 113 characters inline
+          @service('addon-name/-private/do-not-use/the-name-of-the-service')
+          declare someObfuscatedPrivateService: Service;
+
+          // 96 characters
+          @service('addon-name/-private/do-not-use/the-name-of-the-service') declare shorterName: Service;
+
+          // Would be 115 characters inline
+          @service('addon-name/-private/do-not-use/the-name-of-the-service')
+          declare someObfuscatedPrivateService2:! Service;
+        }
+      `,
+      options: [{ printWidth: 100 }],
+    },
+  ],
+  invalid: [
+    {
+      code: stripIndent`
+        export default class LocaleSwitcher extends Component<IArgs> {
+          @service
+          locale!: LocaleService;
+        }
+      `,
+      options: [{ overrides: { 'prefer-inline': ['@service'] } }],
+      errors: [{ message: 'Expected @service to be inline.' }],
+      output: stripIndent`
+        export default class LocaleSwitcher extends Component<IArgs> {
+          @service locale!: LocaleService;
+        }
+      `,
+    },
+  ],
+});

tests/rule-setup.js

@@ -21,10 +21,21 @@ describe('rules setup is correct', function () {
     const path = join(__dirname, '..', 'tests', 'lib', 'rules');
     const files = readdirSync(path);
 
-    // eslint-disable-next-line jest/prefer-strict-equal
-    expect(RULE_NAMES).toEqual(
-      files.filter((file) => !file.startsWith('.')).map((file) => file.replace('.js', ''))
-    );
+    const testFiles = files
+      .filter((file) => !file.startsWith('.'))
+      .map((file) => file.replace('.js', ''));
+
+    for (const rule of RULE_NAMES) {
+      let success = false;
+
+      for (const testFile of testFiles) {
+        if (testFile.startsWith(rule)) {
+          success = true;
+        }
+      }
+
+      expect(success).toBe(true);
+    }
   });
 
   it('should have documentation for all rules', function () {