chore(nx-plugin): review feedback

docs/generated/packages/nx-plugin.json

@@ -50,7 +50,8 @@
             "description": "The tool to use for running lint checks.",
             "type": "string",
             "enum": ["eslint", "tslint"],
-            "default": "eslint"
+            "default": "eslint",
+            "x-deprecated": "TSLint support is deprecated and will be removed"
           },
           "unitTestRunner": {
             "type": "string",
@@ -73,6 +74,11 @@
             "default": false,
             "description": "Do not update tsconfig.json for development experience."
           },
+          "skipLintChecks": {
+            "type": "boolean",
+            "default": false,
+            "description": "Do not eslint configuration for plugin json files."
+          },
           "standaloneConfig": {
             "description": "Split the project configuration into `<projectRoot>/project.json` rather than including it inside `workspace.json`.",
             "type": "boolean"
@@ -318,7 +324,7 @@
     },
     {
       "name": "plugin-lint-checks",
-      "factory": "./src/generators/plugin-lint-checks/generator",
+      "factory": "./src/generators/lint-checks/generator",
       "schema": {
         "$schema": "http://json-schema.org/schema",
         "cli": "nx",
@@ -329,18 +335,18 @@
         "properties": {
           "projectName": {
             "type": "string",
-            "description": "Which project should be the target be added to?",
+            "description": "Which project should be the configuration be added to?",
             "$default": { "$source": "projectName" }
           }
         },
         "required": ["projectName"],
         "presets": []
       },
       "description": "Adds linting configuration to validate common json files for nx plugins.",
-      "implementation": "/packages/nx-plugin/src/generators/plugin-lint-checks/generator.ts",
+      "implementation": "/packages/nx-plugin/src/generators/lint-checks/generator.ts",
       "aliases": [],
       "hidden": false,
-      "path": "/packages/nx-plugin/src/generators/plugin-lint-checks/schema.json"
+      "path": "/packages/nx-plugin/src/generators/lint-checks/schema.json"
     }
   ],
   "executors": [

packages/eslint-plugin-nx/src/rules/nx-plugin-checks.ts

@@ -37,6 +37,7 @@ export type MessageIds =
   | 'missingImplementation'
   | 'invalidImplementationPath'
   | 'invalidImplementationModule'
+  | 'unableToReadImplementationExports'
   | 'invalidVersion'
   | 'missingVersion'
   | 'noGeneratorsOrSchematicsFound'
@@ -60,6 +61,8 @@ export default createESLintRule<Options, MessageIds>({
         '{{ key }}: Implementation path should point to a valid file',
       invalidImplementationModule:
         '{{ key }}: Unable to find export {{ identifier }} in implementation module',
+      unableToReadImplementationExports:
+        '{{ key }}: Unable to read exports for implementation module',
       invalidVersion: '{{ key }}: Version should be a valid semver',
       noGeneratorsOrSchematicsFound:
         'Unable to find `generators` or `schematics` property',
@@ -370,13 +373,23 @@ export function validateImplemenationNode(
     }
 
     if (identifier) {
-      const m = require(resolvedPath);
-      if (!(identifier in m && typeof m[identifier] === 'function')) {
+      try {
+        const m = require(resolvedPath);
+        if (!(identifier in m && typeof m[identifier] === 'function')) {
+          context.report({
+            messageId: 'invalidImplementationModule',
+            node: implementationNode.value as any,
+            data: {
+              identifier,
+              key,
+            },
+          });
+        }
+      } catch {
         context.report({
-          messageId: 'invalidImplementationModule',
+          messageId: 'unableToReadImplementationExports',
           node: implementationNode.value as any,
           data: {
-            identifier,
             key,
           },
         });

packages/nx-plugin/generators.json

@@ -30,8 +30,8 @@
       "description": "Create a executor for an Nx Plugin."
     },
     "plugin-lint-checks": {
-      "factory": "./src/generators/plugin-lint-checks/generator",
-      "schema": "./src/generators/plugin-lint-checks/schema.json",
+      "factory": "./src/generators/lint-checks/generator",
+      "schema": "./src/generators/lint-checks/schema.json",
       "description": "Adds linting configuration to validate common json files for nx plugins."
     }
   },

packages/nx-plugin/src/generators/lint-checks/generator.spec.ts

@@ -0,0 +1,148 @@
+import { createTreeWithEmptyWorkspace } from '@nrwl/devkit/testing';
+import {
+  Tree,
+  readProjectConfiguration,
+  readJson,
+  updateJson,
+  joinPathFragments,
+  writeJson,
+} from '@nrwl/devkit';
+
+import type { Linter as ESLint } from 'eslint';
+import { Schema as EsLintExecutorOptions } from '@nrwl/linter/src/executors/eslint/schema';
+
+import generator from './generator';
+import pluginGenerator from '../plugin/plugin';
+import { Linter } from '@nrwl/linter';
+import { PackageJson } from 'nx/src/utils/package-json';
+
+describe('lint-checks generator', () => {
+  let appTree: Tree;
+
+  beforeEach(async () => {
+    appTree = createTreeWithEmptyWorkspace(2);
+    await pluginGenerator(appTree, {
+      name: 'plugin',
+      importPath: '@acme/plugin',
+      compiler: 'tsc',
+      linter: Linter.EsLint,
+      skipFormat: false,
+      skipTsConfig: false,
+      skipLintChecks: true, // we manually call it s.t. we can update config files first
+      unitTestRunner: 'jest',
+    });
+  });
+
+  it('should update configuration files for default plugin', async () => {
+    await generator(appTree, { projectName: 'plugin' });
+    const projectConfig = readProjectConfiguration(appTree, 'plugin');
+    const targetConfig = projectConfig.targets?.['lint'];
+    const eslintConfig: ESLint.Config = readJson(
+      appTree,
+      `${projectConfig.root}/.eslintrc.json`
+    );
+
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/generators.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/executors.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/package.json`
+    );
+    expect(eslintConfig.overrides).toContainEqual(
+      expect.objectContaining({
+        files: expect.arrayContaining([
+          './executors.json',
+          './package.json',
+          './generators.json',
+        ]),
+        rules: {
+          '@nrwl/nx/nx-plugin-checks': 'error',
+        },
+      })
+    );
+  });
+
+  it('should not duplicate configuration', async () => {
+    await generator(appTree, { projectName: 'plugin' });
+    await generator(appTree, { projectName: 'plugin' });
+    const projectConfig = readProjectConfiguration(appTree, 'plugin');
+    const targetConfig = projectConfig.targets?.['lint']
+      .options as EsLintExecutorOptions;
+    const eslintConfig: ESLint.Config = readJson(
+      appTree,
+      `${projectConfig.root}/.eslintrc.json`
+    );
+
+    const uniqueLintFilePatterns = new Set(targetConfig.lintFilePatterns);
+
+    expect(targetConfig.lintFilePatterns).toHaveLength(
+      uniqueLintFilePatterns.size
+    );
+
+    expect(
+      eslintConfig.overrides.filter(
+        (x) => '@nrwl/nx/nx-plugin-checks' in x.rules
+      )
+    ).toHaveLength(1);
+  });
+
+  it('should update configuration files for angular-style plugin', async () => {
+    const startingProjectConfig = readProjectConfiguration(appTree, 'plugin');
+    updateJson(
+      appTree,
+      joinPathFragments(startingProjectConfig.root, 'package.json'),
+      (json: PackageJson) => {
+        json.schematics = './collection.json';
+        delete json.generators;
+        json.builders = './builders.json';
+        delete json.executors;
+        json['ng-update'] = './migrations.json';
+        return json;
+      }
+    );
+    writeJson(
+      appTree,
+      joinPathFragments(startingProjectConfig.root, 'migrations.json'),
+      {}
+    );
+    await generator(appTree, { projectName: 'plugin' });
+    const projectConfig = readProjectConfiguration(appTree, 'plugin');
+    const targetConfig = projectConfig.targets?.['lint'];
+    const eslintConfig: ESLint.Config = readJson(
+      appTree,
+      `${projectConfig.root}/.eslintrc.json`
+    );
+
+    expect(targetConfig.options.lintFilePatterns).not.toContain(
+      `${projectConfig.root}/generators.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/collection.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).not.toContain(
+      `${projectConfig.root}/executors.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/builders.json`
+    );
+    expect(targetConfig.options.lintFilePatterns).toContain(
+      `${projectConfig.root}/migrations.json`
+    );
+    expect(eslintConfig.overrides).toContainEqual(
+      expect.objectContaining({
+        files: expect.arrayContaining([
+          './collection.json',
+          './package.json',
+          './builders.json',
+          './migrations.json',
+        ]),
+        rules: {
+          '@nrwl/nx/nx-plugin-checks': 'error',
+        },
+      })
+    );
+  });
+});