Rule proposal: prefer-promise-strategies (#245)

* feat(rules): add prefer-promise-strategies

README.md

@@ -86,6 +86,7 @@ Rule                              | Recommended                        | Options
 [no-unsafe-spy][]                 | 1                                  |
 [valid-expect][]                  | `deprecated`                       |
 [prefer-jasmine-matcher][]        | 1                                  |
+[prefer-promise-strategies][]     | 1                                  |
 [prefer-toHaveBeenCalledWith][]   | 1                                  |
 [prefer-toBeUndefined][]          | 0                                  | `['always', 'never']`
 
@@ -131,6 +132,7 @@ See [configuring rules][] for more information.
 [no-unsafe-spy]: docs/rules/no-unsafe-spy.md
 [valid-expect]: docs/rules/valid-expect.md
 [prefer-jasmine-matcher]: docs/rules/prefer-jasmine-matcher.md
+[prefer-promise-strategies]: docs/rules/prefer-promise-strategies.md
 [prefer-toHaveBeenCalledWith]: docs/rules/prefer-toHaveBeenCalledWith.md
 [prefer-toBeUndefined]: docs/rules/prefer-toBeUndefined.md
 

docs/rules/prefer-promise-strategies.md

@@ -0,0 +1,34 @@
+# Prefer Promise Strategies
+
+This rule recommends using `resolveTo(X)` and `rejectWith(X)` instead of `returnValue(Promise.resolve(X))` and `returnValue(Promise.reject(X))`.
+
+Aside from leading to more concise code, transforming `returnValue(Promise.reject(X))` can also avoid an unwanted `UnhandledPromiseRejectionWarning`.
+
+## Rule details
+
+Examples of *incorrect* code:
+
+```js
+const spy = jasmine.createSpy();
+
+spy.withArgs(0).returnValue(Promise.resolve(123));
+spy.and.returnValue(Promise.reject(123));
+```
+
+Examples of *correct* code:
+
+```js
+const spy = jasmine.createSpy();
+
+spy.withArgs(0).resolveTo(123);
+spy.and.rejectWith(123);
+```
+
+## Limitations
+
+To avoid false positives, this rule only considers `returnValue` calls that match the pattern `X.{and,withArgs(Y)}.returnValue(Z)`. Thus, the following *incorrect* code is *not* recognized:
+
+```js
+const strategy = jasmine.createSpy().and;
+strategy.returnValue(Promise.resolve(123));
+```

index.js

@@ -22,6 +22,7 @@ module.exports = {
     'new-line-between-declarations': require('./lib/rules/new-line-between-declarations'),
     'new-line-before-expect': require('./lib/rules/new-line-before-expect'),
     'prefer-jasmine-matcher': require('./lib/rules/prefer-jasmine-matcher'),
+    'prefer-promise-strategies': require('./lib/rules/prefer-promise-strategies'),
     'prefer-toHaveBeenCalledWith': require('./lib/rules/prefer-toHaveBeenCalledWith'),
     'prefer-toBeUndefined': require('./lib/rules/prefer-toBeUndefined')
   },
@@ -47,6 +48,7 @@ module.exports = {
         'jasmine/new-line-between-declarations': 1,
         'jasmine/new-line-before-expect': 1,
         'jasmine/prefer-jasmine-matcher': 1,
+        'jasmine/prefer-promise-strategies': 1,
         'jasmine/prefer-toHaveBeenCalledWith': 1,
         'jasmine/prefer-toBeUndefined': 0
       }

lib/rules/prefer-promise-strategies.js

@@ -0,0 +1,63 @@
+'use strict'
+
+/**
+ * @fileoverview Prefer resolveTo and rejectWith instead of
+ *   returnValue(Promise.{resolve,reject}(X)).
+ * @author Raphael von der Grün
+ */
+
+// Matches X.{and,withArgs(Y)}.returnValue(Z)
+const SpyStrategyCall = `CallExpression:matches(
+  [callee.object.property.name=and],
+  [callee.object.callee.property.name=withArgs]
+)`.replace(/\s+/g, ' ')
+const ReturnStrategyCtor = `${SpyStrategyCall}[callee.property.name=returnValue]`
+
+// Matches Promise.{resolve,reject}(X)
+const PromiseCall = 'CallExpression[callee.object.name=Promise]'
+const SettledPromiseCtor = `${PromiseCall}[callee.property.name=/resolve|reject/]`
+
+/** @type {import("@types/eslint").Rule.RuleModule} */
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    fixable: 'code',
+    schema: []
+  },
+
+  create: context => ({
+    /**
+     * Visits SettledPromiseCtors that are passed as first argument to a ReturnStrategyCtor
+     * @type {import("@types/eslint").Rule.NodeListener['CallExpression']}
+     */
+    [`${ReturnStrategyCtor} > ${SettledPromiseCtor}.arguments:first-child`] (promiseCall) {
+      const returnStrategyCall = promiseCall.parent
+      const returnValueMethod = returnStrategyCall.callee.property
+      const preferredMethod = promiseCall.callee.property.name === 'resolve'
+        ? 'resolveTo' : 'rejectWith'
+
+      context.report({
+        message: `Prefer ${preferredMethod}`,
+
+        // Highlight only the returnValue(…) part, not the entire call
+        loc: {
+          start: returnValueMethod.loc.start,
+          end: returnStrategyCall.loc.end
+        },
+
+        fix (fixer) {
+          const code = context.getSourceCode()
+          return [
+            // Replace Promise constructor call with its arguments
+            fixer.remove(promiseCall.callee),
+            fixer.remove(code.getTokenAfter(promiseCall.callee)),
+            fixer.remove(code.getLastToken(promiseCall)),
+
+            // Replace returnValue method with resolveTo or rejectWith
+            fixer.replaceText(returnValueMethod, preferredMethod)
+          ]
+        }
+      })
+    }
+  })
+}

test/rules/prefer-promise-strategies.js

@@ -0,0 +1,52 @@
+'use strict'
+
+const rule = require('../../lib/rules/prefer-promise-strategies')
+const { RuleTester } = require('eslint')
+
+const parserOptions = { ecmaVersion: 6 }
+const eslintTester = new RuleTester({ parserOptions })
+
+eslintTester.run('prefer-promise-strategies', rule, {
+  valid: [
+    { code: 'spy.and.returnValue(Promise.foo());' },
+    { code: 'spy.and.returnValue();' },
+    { code: 'spy.and.returnValue(123);' },
+    { code: 'spy.and.returnValue(foo());' },
+    { code: 'obj.returnValue(Promise.reject());' },
+    { code: 'spy.and.returnValue(fn => fn(Promise.resolve(123)));' },
+    {
+      // Should be invalid but would need more complex analysis
+      code: 'const s = spy.and; s.returnValue(Promise.resolve(123));'
+    }
+  ],
+  invalid: [
+    {
+      code: 'spy.withArgs(0).returnValue(Promise.resolve(123));',
+      output: 'spy.withArgs(0).resolveTo(123);',
+      errors: [
+        { message: 'Prefer resolveTo' }
+      ]
+    },
+    {
+      code: 'spy.and.returnValue(Promise.reject(123));',
+      output: 'spy.and.rejectWith(123);',
+      errors: [
+        { message: 'Prefer rejectWith' }
+      ]
+    },
+    { // Handles empty argument list
+      code: 'spy.and.returnValue(Promise.resolve());',
+      output: 'spy.and.resolveTo();',
+      errors: [
+        { message: 'Prefer resolveTo' }
+      ]
+    },
+    { // Handles multiple arguments (even if invalid)
+      code: 'spy.and.returnValue(Promise.resolve(1,2, 3));',
+      output: 'spy.and.resolveTo(1,2, 3);',
+      errors: [
+        { message: 'Prefer resolveTo' }
+      ]
+    }
+  ]
+})