fix(#195): decorators using declare now respect prettier

Additionally, - deleted most of the length detection code - new smoke test - some general cleanup
NullVoxPopuli · Nov 9, 2020 · 607f4ba · 607f4ba
1 parent 979e2f3
commit 607f4ba
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 55 deletions.
diff --git a/lib/rules/decorator-position.js b/lib/rules/decorator-position.js
@@ -204,28 +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 hasDeclare = node.declare === true;
-    let token = context.getSourceCode().getTokenAfter(decorator, { includeComments: true });
-    let declareToken;
-
-    if (hasDeclare) {
-      declareToken = token;
-      token = context.getSourceCode().getTokenAfter(token, { includeComments: true });
-    }
-
-    const whitespaceStart = decorator.range[1];
-    const whitespaceEnd = token.range[0];
 
-    const whitespaceLength = whitespaceEnd - whitespaceStart;
-
-    const totalLineLength = calculateTotalLineLength(context, node, token, declareToken) + 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 = {
@@ -235,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;
@@ -266,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];
 
@@ -299,25 +288,6 @@ function placeDecoratorsAboveProperty(context, node, options) {
 // Helpers
 // ///////////////////////////////////
 
-function calculateTotalLineLength(context, node, token, declareToken) {
-  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;
-
-  let length = decEnd - decStart + tokenEnd - tokenStart + puncEnd - puncStart;
-
-  if (declareToken) {
-    const [declareStart, declareEnd] = declareToken.range;
-
-    length = length + declareEnd - declareStart;
-  }
-
-  return length;
-}
-
 function normalizeOptions(userOptions) {
   const options = Object.assign({}, defaultOptions, userOptions);
 
@@ -390,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;
@@ -401,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;
   }
 
@@ -428,6 +421,8 @@ function decoratorInfo(node, config) {
     arityMatches,
     currentPositionMatchesIntent,
     needsTransform,
+    inlineLength,
+    ifInlineWouldViolatePrettier,
     name: decoratorName,
     ...positioning,
   };

diff --git a/smoke-tests/-rules/position-prettier/typescript.ts b/smoke-tests/-rules/position-prettier/typescript.ts
diff --git a/smoke-tests/issue-reproductions/195/.eslintrc.js b/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',
+      },
+    ],
+  },
+};
diff --git a/smoke-tests/issue-reproductions/195/app.ts b/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;
+}
diff --git a/smoke-tests/issue-reproductions/195/package.json b/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": "*"
+  }
+}
diff --git a/tests/lib/rules/decorator-position-ts.js b/tests/lib/rules/decorator-position-ts.js
@@ -29,11 +29,16 @@ tsRuleTester.run('TS: decorator-position', rule, {
     {
       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 shorterName: Service;
+          declare someObfuscatedPrivateService2:! Service;
         }
       `,
       options: [{ printWidth: 100 }],