Merge pull request #1292 from cucumber/html-circular-dep-fix

Remove circular imports in typescript code, add linting
cucumber · Feb 5, 2021 · cf0aa29 · cf0aa29
2 parents b218f87 + 6ebbf91
commit cf0aa29
Show file tree

Hide file tree

Showing 94 changed files with 648 additions and 473 deletions.
diff --git a/.templates/javascript/.eslintrc.json b/.templates/javascript/.eslintrc.json
@@ -10,17 +10,20 @@
   },
   "plugins": [
     "react",
+    "import",
     "node",
     "@typescript-eslint"
   ],
   "extends": [
     "eslint:recommended",
     "plugin:react/recommended",
+    "plugin:import/typescript",
     "plugin:@typescript-eslint/eslint-recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:prettier/recommended"
   ],
   "rules": {
+    "import/no-cycle": "error",
     "node/no-extraneous-import": "error",
     "@typescript-eslint/ban-ts-ignore": "off",
     "@typescript-eslint/ban-ts-comment": "off",

diff --git a/c21e/javascript/.eslintrc.json b/c21e/javascript/.eslintrc.json
@@ -10,17 +10,20 @@
   },
   "plugins": [
     "react",
+    "import",
     "node",
     "@typescript-eslint"
   ],
   "extends": [
     "eslint:recommended",
     "plugin:react/recommended",
+    "plugin:import/typescript",
     "plugin:@typescript-eslint/eslint-recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:prettier/recommended"
   ],
   "rules": {
+    "import/no-cycle": "error",
     "node/no-extraneous-import": "error",
     "@typescript-eslint/ban-ts-ignore": "off",
     "@typescript-eslint/ban-ts-comment": "off",

diff --git a/c21e/javascript/package.json b/c21e/javascript/package.json
@@ -32,6 +32,7 @@
     "@typescript-eslint/parser": "^4.6.1",
     "eslint": "^7.12.1",
     "eslint-config-prettier": "^7.0.0",
+    "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.1.4",
     "eslint-plugin-react": "^7.21.5",

diff --git a/compatibility-kit/javascript/.eslintrc.json b/compatibility-kit/javascript/.eslintrc.json
@@ -10,17 +10,20 @@
   },
   "plugins": [
     "react",
+    "import",
     "node",
     "@typescript-eslint"
   ],
   "extends": [
     "eslint:recommended",
     "plugin:react/recommended",
+    "plugin:import/typescript",
     "plugin:@typescript-eslint/eslint-recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:prettier/recommended"
   ],
   "rules": {
+    "import/no-cycle": "error",
     "node/no-extraneous-import": "error",
     "@typescript-eslint/ban-ts-ignore": "off",
     "@typescript-eslint/ban-ts-comment": "off",

diff --git a/compatibility-kit/javascript/package.json b/compatibility-kit/javascript/package.json
@@ -22,6 +22,7 @@
     "@typescript-eslint/parser": "^4.6.1",
     "eslint": "^7.12.1",
     "eslint-config-prettier": "^7.0.0",
+    "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.1.4",
     "eslint-plugin-react": "^7.21.5",

diff --git a/create-meta/javascript/.eslintrc.json b/create-meta/javascript/.eslintrc.json
@@ -10,17 +10,20 @@
   },
   "plugins": [
     "react",
+    "import",
     "node",
     "@typescript-eslint"
   ],
   "extends": [
     "eslint:recommended",
     "plugin:react/recommended",
+    "plugin:import/typescript",
     "plugin:@typescript-eslint/eslint-recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:prettier/recommended"
   ],
   "rules": {
+    "import/no-cycle": "error",
     "node/no-extraneous-import": "error",
     "@typescript-eslint/ban-ts-ignore": "off",
     "@typescript-eslint/ban-ts-comment": "off",

diff --git a/create-meta/javascript/package.json b/create-meta/javascript/package.json
@@ -36,6 +36,7 @@
     "@typescript-eslint/parser": "^4.6.1",
     "eslint": "^7.12.1",
     "eslint-config-prettier": "^7.0.0",
+    "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.1.4",
     "eslint-plugin-react": "^7.21.5",

diff --git a/cucumber-expressions/javascript/.eslintrc.json b/cucumber-expressions/javascript/.eslintrc.json
@@ -10,17 +10,20 @@
   },
   "plugins": [
     "react",
+    "import",
     "node",
     "@typescript-eslint"
   ],
   "extends": [
     "eslint:recommended",
     "plugin:react/recommended",
+    "plugin:import/typescript",
     "plugin:@typescript-eslint/eslint-recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:prettier/recommended"
   ],
   "rules": {
+    "import/no-cycle": "error",
     "node/no-extraneous-import": "error",
     "@typescript-eslint/ban-ts-ignore": "off",
     "@typescript-eslint/ban-ts-comment": "off",

diff --git a/cucumber-expressions/javascript/package.json b/cucumber-expressions/javascript/package.json
@@ -36,9 +36,11 @@
     "@typescript-eslint/parser": "^4.9.1",
     "eslint": "^7.15.0",
     "eslint-config-prettier": "^7.0.0",
+    "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.2.0",
     "eslint-plugin-react": "^7.21.5",
+    "js-yaml": "^4.0.0",
     "mocha": "^8.2.1",
     "nyc": "^15.1.0",
     "prettier": "^2.2.1",

diff --git a/cucumber-expressions/javascript/src/Argument.ts b/cucumber-expressions/javascript/src/Argument.ts
@@ -1,7 +1,7 @@
 import TreeRegexp from './TreeRegexp'
 import ParameterType from './ParameterType'
 import Group from './Group'
-import { CucumberExpressionError } from './Errors'
+import CucumberExpressionError from './CucumberExpressionError'
 
 export default class Argument<T> {
   public static build(

diff --git a/cucumber-expressions/javascript/src/CucumberExpression.ts b/cucumber-expressions/javascript/src/CucumberExpression.ts
@@ -9,11 +9,11 @@ import {
   createOptionalMayNotBeEmpty,
   createParameterIsNotAllowedInOptional,
   createUndefinedParameterType,
-  CucumberExpressionError,
 } from './Errors'
 import Expression from './Expression'
 import CucumberExpressionParser from './CucumberExpressionParser'
 import { Node, NodeType } from './Ast'
+import CucumberExpressionError from './CucumberExpressionError'
 
 const ESCAPE_PATTERN = () => /([\\^[({$.|?*+})\]])/g
 

diff --git a/cucumber-expressions/javascript/src/CucumberExpressionError.ts b/cucumber-expressions/javascript/src/CucumberExpressionError.ts
@@ -0,0 +1 @@
+export default class CucumberExpressionError extends Error {}
diff --git a/cucumber-expressions/javascript/src/CucumberExpressionGenerator.ts b/cucumber-expressions/javascript/src/CucumberExpressionGenerator.ts
@@ -1,4 +1,3 @@
-import ParameterTypeRegistry from './ParameterTypeRegistry'
 import ParameterTypeMatcher from './ParameterTypeMatcher'
 import ParameterType from './ParameterType'
 
@@ -7,7 +6,9 @@ import CombinatorialGeneratedExpressionFactory from './CombinatorialGeneratedExp
 import GeneratedExpression from './GeneratedExpression'
 
 export default class CucumberExpressionGenerator {
-  constructor(private readonly parameterTypeRegistry: ParameterTypeRegistry) {}
+  constructor(
+    private readonly parameterTypes: () => Iterable<ParameterType<any>>
+  ) {}
 
   public generateExpressions(text: string): ReadonlyArray<GeneratedExpression> {
     const parameterTypeCombinations: Array<Array<ParameterType<any>>> = []
@@ -90,7 +91,7 @@ export default class CucumberExpressionGenerator {
 
   private createParameterTypeMatchers(text: string): ParameterTypeMatcher[] {
     let parameterMatchers: ParameterTypeMatcher[] = []
-    for (const parameterType of this.parameterTypeRegistry.parameterTypes) {
+    for (const parameterType of this.parameterTypes()) {
       if (parameterType.useForSnippets) {
         parameterMatchers = parameterMatchers.concat(
           CucumberExpressionGenerator.createParameterTypeMatchers2(

diff --git a/cucumber-expressions/javascript/src/Errors.ts b/cucumber-expressions/javascript/src/Errors.ts
@@ -1,8 +1,7 @@
 import ParameterType from './ParameterType'
 import GeneratedExpression from './GeneratedExpression'
 import { Located, Node, purposeOf, symbolOf, Token, TokenType } from './Ast'
-
-export class CucumberExpressionError extends Error {}
+import CucumberExpressionError from './CucumberExpressionError'
 
 export function createAlternativeMayNotExclusivelyContainOptionals(
   node: Node,
@@ -138,12 +137,6 @@ export function createCantEscaped(expression: string, index: number) {
   )
 }
 
-export function createInvalidParameterTypeName(typeName: string) {
-  return new CucumberExpressionError(
-    `Illegal character in parameter name {${typeName}}. Parameter names may not contain '{', '}', '(', ')', '\\' or '/'`
-  )
-}
-
 export function createInvalidParameterTypeNameInNode(
   token: Token,
   expression: string

diff --git a/cucumber-expressions/javascript/src/ParameterType.ts b/cucumber-expressions/javascript/src/ParameterType.ts
@@ -1,7 +1,4 @@
-import {
-  createInvalidParameterTypeName,
-  CucumberExpressionError,
-} from './Errors'
+import CucumberExpressionError from './CucumberExpressionError'
 
 const ILLEGAL_PARAMETER_NAME_PATTERN = /([[\]()$.|?*+])/
 const UNESCAPE_PATTERN = () => /(\\([[$.|?*+\]]))/g
@@ -21,7 +18,9 @@ export default class ParameterType<T> {
 
   public static checkParameterTypeName(typeName: string) {
     if (!this.isValidParameterTypeName(typeName)) {
-      throw createInvalidParameterTypeName(typeName)
+      throw new CucumberExpressionError(
+        `Illegal character in parameter name {${typeName}}. Parameter names may not contain '{', '}', '(', ')', '\\' or '/'`
+      )
     }
   }
 

diff --git a/cucumber-expressions/javascript/src/ParameterTypeRegistry.ts b/cucumber-expressions/javascript/src/ParameterTypeRegistry.ts
@@ -1,7 +1,8 @@
 import ParameterType from './ParameterType'
 
 import CucumberExpressionGenerator from './CucumberExpressionGenerator'
-import { AmbiguousParameterTypeError, CucumberExpressionError } from './Errors'
+import { AmbiguousParameterTypeError } from './Errors'
+import CucumberExpressionError from './CucumberExpressionError'
 
 export default class ParameterTypeRegistry {
   public static readonly INTEGER_REGEXPS = [/-?\d+/, /\d+/]
@@ -69,7 +70,7 @@ export default class ParameterTypeRegistry {
     )
   }
 
-  get parameterTypes() {
+  get parameterTypes(): IterableIterator<ParameterType<any>> {
     return this.parameterTypeByName.values()
   }
 
@@ -88,10 +89,10 @@ export default class ParameterTypeRegistry {
     }
     if (parameterTypes.length > 1 && !parameterTypes[0].preferForRegexpMatch) {
       // We don't do this check on insertion because we only want to restrict
-      // ambiguiuty when we look up by Regexp. Users of CucumberExpression should
+      // ambiguity when we look up by Regexp. Users of CucumberExpression should
       // not be restricted.
       const generatedExpressions = new CucumberExpressionGenerator(
-        this
+        () => this.parameterTypes
       ).generateExpressions(text)
       throw AmbiguousParameterTypeError.forRegExp(
         parameterTypeRegexp,

diff --git a/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts b/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts
@@ -39,12 +39,16 @@ describe('CucumberExpressionGenerator', () => {
 
   beforeEach(() => {
     parameterTypeRegistry = new ParameterTypeRegistry()
-    generator = new CucumberExpressionGenerator(parameterTypeRegistry)
+    generator = new CucumberExpressionGenerator(
+      () => parameterTypeRegistry.parameterTypes
+    )
   })
 
   it('documents expression generation', () => {
     parameterTypeRegistry = new ParameterTypeRegistry()
-    generator = new CucumberExpressionGenerator(parameterTypeRegistry)
+    generator = new CucumberExpressionGenerator(
+      () => parameterTypeRegistry.parameterTypes
+    )
     const undefinedStepText = 'I have 2 cucumbers and 1.5 tomato'
     const generatedExpression = generator.generateExpressions(
       undefinedStepText
@@ -267,7 +271,7 @@ describe('CucumberExpressionGenerator', () => {
 
     const expressions = generator.generateExpressions('I download a picture')
     assert.strictEqual(expressions.length, 1)
-    assert.notEqual(expressions[0].source, 'I {direction}load a picture')
+    assert.notStrictEqual(expressions[0].source, 'I {direction}load a picture')
     assert.strictEqual(expressions[0].source, 'I download a picture')
   })
 
@@ -278,7 +282,10 @@ describe('CucumberExpressionGenerator', () => {
 
     const expressions = generator.generateExpressions('I watch the muppet show')
     assert.strictEqual(expressions.length, 1)
-    assert.notEqual(expressions[0].source, 'I watch the m{direction}pet show')
+    assert.notStrictEqual(
+      expressions[0].source,
+      'I watch the m{direction}pet show'
+    )
     assert.strictEqual(expressions[0].source, 'I watch the muppet show')
   })
 
@@ -289,7 +296,7 @@ describe('CucumberExpressionGenerator', () => {
 
     const expressions = generator.generateExpressions('I create a group')
     assert.strictEqual(expressions.length, 1)
-    assert.notEqual(expressions[0].source, 'I create a gro{direction}')
+    assert.notStrictEqual(expressions[0].source, 'I create a gro{direction}')
     assert.strictEqual(expressions[0].source, 'I create a group')
   })
 

diff --git a/cucumber-expressions/javascript/test/CucumberExpressionParserTest.ts b/cucumber-expressions/javascript/test/CucumberExpressionParserTest.ts
@@ -1,9 +1,8 @@
 import fs from 'fs'
-// eslint-disable-next-line node/no-extraneous-import
-import yaml from 'js-yaml' // why?
+import yaml from 'js-yaml'
 import assert from 'assert'
-import { CucumberExpressionError } from '../src/Errors'
 import CucumberExpressionParser from '../src/CucumberExpressionParser'
+import CucumberExpressionError from '../src/CucumberExpressionError'
 
 interface Expectation {
   expression: string
@@ -14,7 +13,7 @@ interface Expectation {
 describe('Cucumber expression parser', () => {
   fs.readdirSync('testdata/ast').forEach((testcase) => {
     const testCaseData = fs.readFileSync(`testdata/ast/${testcase}`, 'utf-8')
-    const expectation = yaml.safeLoad(testCaseData) as Expectation
+    const expectation = yaml.load(testCaseData) as Expectation
     it(`${testcase}`, () => {
       const parser = new CucumberExpressionParser()
       if (expectation.exception == undefined) {

diff --git a/cucumber-expressions/javascript/test/CucumberExpressionTest.ts b/cucumber-expressions/javascript/test/CucumberExpressionTest.ts
@@ -3,9 +3,8 @@ import CucumberExpression from '../src/CucumberExpression'
 import ParameterTypeRegistry from '../src/ParameterTypeRegistry'
 import ParameterType from '../src/ParameterType'
 import fs from 'fs'
-// eslint-disable-next-line node/no-extraneous-import
-import yaml from 'js-yaml' // why?
-import { CucumberExpressionError } from '../src/Errors'
+import yaml from 'js-yaml'
+import CucumberExpressionError from '../src/CucumberExpressionError'
 
 interface Expectation {
   expression: string
@@ -20,7 +19,7 @@ describe('CucumberExpression', () => {
       `testdata/expression/${testcase}`,
       'utf-8'
     )
-    const expectation = yaml.safeLoad(testCaseData) as Expectation
+    const expectation = yaml.load(testCaseData) as Expectation
     it(`${testcase}`, () => {
       const parameterTypeRegistry = new ParameterTypeRegistry()
       if (expectation.exception == undefined) {
@@ -51,7 +50,7 @@ describe('CucumberExpression', () => {
 
   fs.readdirSync('testdata/regex').forEach((testcase) => {
     const testCaseData = fs.readFileSync(`testdata/regex/${testcase}`, 'utf-8')
-    const expectation = yaml.safeLoad(testCaseData) as Expectation
+    const expectation = yaml.load(testCaseData) as Expectation
     it(`${testcase}`, () => {
       const parameterTypeRegistry = new ParameterTypeRegistry()
       const expression = new CucumberExpression(

diff --git a/cucumber-expressions/javascript/test/CucumberExpressionTokenizerTest.ts b/cucumber-expressions/javascript/test/CucumberExpressionTokenizerTest.ts
@@ -1,9 +1,8 @@
 import fs from 'fs'
-// eslint-disable-next-line node/no-extraneous-import
-import yaml from 'js-yaml' // why?
+import yaml from 'js-yaml'
 import CucumberExpressionTokenizer from '../src/CucumberExpressionTokenizer'
 import assert from 'assert'
-import { CucumberExpressionError } from '../src/Errors'
+import CucumberExpressionError from '../src/CucumberExpressionError'
 
 interface Expectation {
   expression: string
@@ -14,7 +13,7 @@ interface Expectation {
 describe('Cucumber expression tokenizer', () => {
   fs.readdirSync('testdata/tokens').forEach((testcase) => {
     const testCaseData = fs.readFileSync(`testdata/tokens/${testcase}`, 'utf-8')
-    const expectation = yaml.safeLoad(testCaseData) as Expectation
+    const expectation = yaml.load(testCaseData) as Expectation
     it(`${testcase}`, () => {
       const tokenizer = new CucumberExpressionTokenizer()
       if (expectation.exception == undefined) {