Merge pull request #29 from buildo/no-module-imports

Add no-module-imports

Author: gabro

README.md

@@ -37,15 +37,17 @@ and enable the rules you want, for example
 ## List of supported rules
 
 - [fp-ts/no-lib-imports](docs/rules/no-lib-imports.md): Disallow imports from
-  'fp-ts/lib'
+  'fp-ts/lib' (autofixable 🔧)
 - [fp-ts/no-pipeable](docs/rules/no-pipeable.md): Disallow imports from the
-  'pipeable' module
+  'pipeable' module (autofixable 🔧)
 - [fp-ts/prefer-traverse](docs/rules/prefer-traverse.md): Replace map + sequence
-  with traverse
+  with traverse (autofixable 🔧)
 - [fp-ts/no-redundant-flow](docs/rules/no-redundant-flow.md): Remove redundant
-  uses of flow
+  uses of flow (autofixable 🔧)
 - [fp-ts/prefer-chain](docs/rules/prefer-chain.md): Replace map + flatten with
-  chain
+  chain (autofixable 🔧)
+- [fp-ts/no-module-imports](docs/rules/no-module-imports.md): Disallow imports
+  from fp-ts modules (autofixable 🔧)
 
 ## Configurations
 

docs/rules/no-module-imports.md

@@ -0,0 +1,50 @@
+# Disallow imports from fp-ts modules (fp-ts/no-module-imports)
+
+Disallow imports from fp-ts modules, such as `fp-ts/Option`.
+
+The `function` module is an exception and it's allowed nonetheless, since it's
+not exported from fp-ts's index.
+
+**Fixable**: This rule is automatically fixable using the `--fix` flag on the
+command line.
+
+## Rule Details
+
+Possible configurations:
+
+- `"always"`: disallow importing any member from a module. This is the default
+  value.
+- `"allow-types"`: allow importing type members from a module.
+
+Example of **incorrect** code for this rule, when configured as `always`:
+
+```ts
+import { Option, some } from "fp-ts/Option";
+
+const x: Option<number> = some(42);
+```
+
+Example of **incorrect** code for this rule, when configured as `allow-types`:
+
+```ts
+import { some } from "fp-ts/Option";
+
+const x = some(42);
+```
+
+Example of **correct** code for this rule, when configured as `always`:
+
+```ts
+import { option } from "fp-ts";
+
+const x: option.Option<number> = option.some(42);
+```
+
+Example of **correct** code for this rule, when configured as `allow-types`:
+
+```ts
+import { option } from "fp-ts";
+import { Option } from "fp-ts/Option";
+
+const x: Option<number> = option.some(42);
+```

package.json

@@ -26,13 +26,17 @@
   "dependencies": {
     "@typescript-eslint/experimental-utils": "^4.11.1",
     "astring": "^1.4.3",
+    "estraverse": "^5.2.0",
     "fp-ts": "^2.9.3"
   },
   "devDependencies": {
     "@types/astring": "^1.3.0",
+    "@types/common-tags": "^1.8.0",
+    "@types/estraverse": "^5.1.0",
     "@types/node": "^14.14.16",
     "@types/requireindex": "^1.2.0",
     "@typescript-eslint/parser": "^4.11.1",
+    "common-tags": "^1.8.0",
     "eslint": "^7.17.0",
     "eslint-plugin-fp-ts": "^0.1.8",
     "jest": "^26.6.3",

src/index.ts

@@ -4,6 +4,7 @@ import { pipe } from "fp-ts/function";
 const potentialErrors = {
   "no-lib-imports": require("./rules/no-lib-imports"),
   "no-pipeable": require("./rules/no-pipeable"),
+  "no-module-imports": require("./rules/no-module-imports"),
 };
 
 const suggestions = {

src/rules/no-lib-imports.ts

@@ -1,4 +1,4 @@
-import { ASTUtils, TSESLint } from "@typescript-eslint/experimental-utils";
+import { TSESLint } from "@typescript-eslint/experimental-utils";
 
 const messages = {
   importNotAllowed:
@@ -18,7 +18,7 @@ export function create(
 ): TSESLint.RuleListener {
   return {
     ImportDeclaration(node) {
-      const sourceValue = ASTUtils.getStringIfConstant(node.source);
+      const sourceValue = node.source.value?.toString();
       if (sourceValue) {
         const forbiddenImportPattern = /^fp-ts\/lib\//;
 

src/rules/no-module-imports.ts

@@ -0,0 +1,151 @@
+import {
+  ASTUtils,
+  AST_NODE_TYPES,
+  TSESLint,
+} from "@typescript-eslint/experimental-utils";
+import { array } from "fp-ts";
+import { pipe } from "fp-ts/function";
+import { contextUtils } from "../utils";
+
+const messages = {
+  importNotAllowed:
+    "Importing from modules is not allowed, import from 'fp-ts' instead",
+  importValuesNotAllowed:
+    "Importing values from modules is not allowed, import from 'fp-ts' instead",
+  convertImportToIndex: "Import all members from fp-ts",
+  convertImportValuesToIndex: "Import values from fp-ts",
+} as const;
+type MessageIds = keyof typeof messages;
+
+type Options = ["always" | "allow-types"];
+
+export const meta: TSESLint.RuleMetaData<MessageIds> = {
+  type: "suggestion",
+  fixable: "code",
+  schema: [
+    {
+      enum: ["always", "allow-types"],
+    },
+  ],
+  messages,
+};
+
+export function create(
+  context: TSESLint.RuleContext<MessageIds, Options>
+): TSESLint.RuleListener {
+  const allowTypes = context.options[0] === "allow-types";
+  const allowedModules = ["function"];
+
+  const {
+    addNamedImportIfNeeded,
+    isOnlyUsedAsType,
+    removeImportDeclaration,
+  } = contextUtils(context);
+
+  return {
+    ImportDeclaration(node) {
+      const sourceValue = node.source.value?.toString();
+      if (sourceValue) {
+        const forbiddenImportPattern = /^fp-ts\/(.+)/;
+        const matches = sourceValue.match(forbiddenImportPattern);
+
+        if (matches != null) {
+          const matchedModule = matches[1]!.replace("lib/", "");
+          if (allowedModules.includes(matchedModule)) {
+            return;
+          }
+
+          const importSpecifiers = node.specifiers.filter(
+            (importClause) =>
+              importClause.type === AST_NODE_TYPES.ImportSpecifier
+          );
+
+          const nonTypeImports = pipe(
+            importSpecifiers,
+            array.filter((i) => !isOnlyUsedAsType(i))
+          );
+
+          if (allowTypes && nonTypeImports.length === 0) {
+            return;
+          }
+
+          if (importSpecifiers.length > 0) {
+            context.report({
+              node: node.source,
+              messageId: allowTypes
+                ? "importValuesNotAllowed"
+                : "importNotAllowed",
+              suggest: [
+                {
+                  messageId: allowTypes
+                    ? "convertImportValuesToIndex"
+                    : "convertImportToIndex",
+                  fix(fixer) {
+                    const indexExport =
+                      matchedModule.charAt(0).toLowerCase() +
+                      matchedModule.slice(1);
+
+                    const referencesFixes = importSpecifiers.flatMap(
+                      (importSpecifier) => {
+                        const variable = ASTUtils.findVariable(
+                          context.getScope(),
+                          importSpecifier.local.name
+                        );
+                        if (variable) {
+                          return variable.references
+                            .filter((ref) =>
+                              allowTypes
+                                ? ref.identifier.parent?.type !==
+                                  AST_NODE_TYPES.TSTypeReference
+                                : true
+                            )
+                            .filter(
+                              (ref) =>
+                                ref.identifier.parent?.type !==
+                                AST_NODE_TYPES.MemberExpression
+                            )
+                            .map((ref) =>
+                              fixer.insertTextBefore(
+                                ref.identifier,
+                                `${indexExport}.`
+                              )
+                            );
+                        } else {
+                          return [];
+                        }
+                      }
+                    );
+
+                    const importFixes =
+                      !allowTypes ||
+                      nonTypeImports.length === importSpecifiers.length
+                        ? [removeImportDeclaration(node, fixer)]
+                        : nonTypeImports.map((node) => {
+                            if (
+                              context.getSourceCode().getTokenAfter(node)
+                                ?.value === ","
+                            ) {
+                              return fixer.removeRange([
+                                node.range[0],
+                                node.range[1] + 1,
+                              ]);
+                            } else {
+                              return fixer.remove(node);
+                            }
+                          });
+
+                    return [
+                      ...importFixes,
+                      ...addNamedImportIfNeeded(indexExport, "fp-ts", fixer),
+                      ...referencesFixes,
+                    ];
+                  },
+                },
+              ],
+            });
+          }
+        }
+      }
+    },
+  };
+}

src/rules/no-redundant-flow.ts

@@ -1,5 +1,5 @@
 import { TSESLint } from "@typescript-eslint/experimental-utils";
-import { isFlowExpression } from "../utils";
+import { contextUtils } from "../utils";
 
 const messages = {
   redundantFlow: "flow can be removed because it takes only one argument",
@@ -17,9 +17,11 @@ export const meta: TSESLint.RuleMetaData<MessageIds> = {
 export function create(
   context: TSESLint.RuleContext<MessageIds, []>
 ): TSESLint.RuleListener {
+  const { isFlowExpression } = contextUtils(context);
+
   return {
     CallExpression(node) {
-      if (node.arguments.length === 1 && isFlowExpression(node, context)) {
+      if (node.arguments.length === 1 && isFlowExpression(node)) {
         context.report({
           node,
           messageId: "redundantFlow",