From 13fe6eeccf31d8460502a4ec841a8b57af9caab0 Mon Sep 17 00:00:00 2001 From: Blake Thomas Williams <49404493+btw17@users.noreply.github.com> Date: Fri, 10 Feb 2023 11:12:18 -0600 Subject: [PATCH] feat: added `tests/typescript` to test supported TS examples (#6775) * feat: added `tests/typescript` to test supported TS examples * fix: update the test name, description, and output * chore: remove unused imports in `test_tasks.js` * fix: wrap README line at 80 characters * fix: implemented `different_user_input.ts` feedback * fix: correct mistaken comments * chore: rename `./eslintrc.json` to `./eslintrc.js` * feat: added linting for tests/typescript * chore: cleanup eslintrc lines over 80 characters * fix: updated `.eslintrc.js` to provide an override for linting itself * fix: updated tests to build to the `build` directory * feat: updated `gulp format` to handle formatting `.eslintrc.js` * fix: updated `.eslintrc.js` to align with both formatter and linter * fix: updated config comment wording * fix: removed quotes for valid identifiers * Revert "fix: removed quotes for valid identifiers" This reverts commit 03eff91aea1468e503bc79a90fb139914d3f39d2. --- .eslintrc.js | 221 ++++++++++++++++++ .eslintrc.json | 165 ------------- scripts/gulpfiles/build_tasks.js | 3 +- scripts/gulpfiles/config.js | 3 + scripts/gulpfiles/test_tasks.js | 14 +- tests/mocha/.eslintrc.json | 2 +- tests/node/.eslintrc.json | 2 +- tests/typescript/README.md | 4 + .../src/field/different_user_input.ts | 82 +++++++ tests/typescript/tsconfig.json | 22 ++ 10 files changed, 348 insertions(+), 170 deletions(-) create mode 100644 .eslintrc.js delete mode 100644 .eslintrc.json create mode 100644 tests/typescript/README.md create mode 100644 tests/typescript/src/field/different_user_input.ts create mode 100644 tests/typescript/tsconfig.json diff --git a/.eslintrc.js b/.eslintrc.js new file mode 100644 index 000000000..e92f3d8c6 --- /dev/null +++ b/.eslintrc.js @@ -0,0 +1,221 @@ +const rules = { + 'curly': ['error'], + 'eol-last': ['error'], + 'keyword-spacing': ['error'], + 'linebreak-style': ['error', 'unix'], + 'max-len': [ + 'error', + { + 'code': 100, + 'tabWidth': 4, + 'ignoreStrings': true, + 'ignoreRegExpLiterals': true, + 'ignoreUrls': true, + }, + ], + 'no-trailing-spaces': ['error', {'skipBlankLines': true}], + 'no-unused-vars': [ + 'warn', + { + 'args': 'after-used', + // Ignore vars starting with an underscore. + 'varsIgnorePattern': '^_', + // Ignore arguments starting with an underscore. + 'argsIgnorePattern': '^_', + }, + ], + // Blockly uses for exporting symbols. no-self-assign added in eslint 5. + 'no-self-assign': ['off'], + // Blockly uses single quotes except for JSON blobs, which must use double + // quotes. + 'quotes': ['off'], + 'semi': ['error', 'always'], + // Blockly doesn't have space before function paren when defining functions. + 'space-before-function-paren': ['error', 'never'], + // Blockly doesn't have space before function paren when calling functions. + 'func-call-spacing': ['error', 'never'], + 'space-infix-ops': ['error'], + // Blockly uses 'use strict' in files. + 'strict': ['off'], + // Closure style allows redeclarations. + 'no-redeclare': ['off'], + 'valid-jsdoc': ['error'], + 'no-console': ['off'], + 'no-multi-spaces': ['error', {'ignoreEOLComments': true}], + 'operator-linebreak': ['error', 'after'], + 'spaced-comment': [ + 'error', + 'always', + { + 'block': { + 'balanced': true, + }, + 'exceptions': ['*'], + }, + ], + // Blockly uses prefixes for optional arguments and test-only functions. + 'camelcase': [ + 'error', + { + 'properties': 'never', + 'allow': ['^opt_', '^_opt_', '^testOnly_'], + }, + ], + // Use clang-format for indentation by running `npm run format`. + 'indent': ['off'], + // Blockly uses capital letters for some non-constructor namespaces. + // Keep them for legacy reasons. + 'new-cap': ['off'], + // Mostly use default rules for brace style, but allow single-line blocks. + 'brace-style': ['error', '1tbs', {'allowSingleLine': true}], + // Blockly uses objects as maps, but uses Object.create(null) to + // instantiate them. + 'guard-for-in': ['off'], + 'prefer-spread': ['off'], + 'comma-dangle': [ + 'error', + { + 'arrays': 'always-multiline', + 'objects': 'always-multiline', + 'imports': 'always-multiline', + 'exports': 'always-multiline', + 'functions': 'ignore', + }, + ], +}; + +/** + * Build shared settings for TS linting and add in the config differences. + * @return {Object} The override TS linting for given files and a given + * tsconfig. + */ +function buildTSOverride({files, tsconfig}) { + return { + 'files': files, + 'plugins': [ + '@typescript-eslint/eslint-plugin', + 'jsdoc', + ], + 'settings': { + 'jsdoc': { + 'mode': 'typescript', + }, + }, + 'parser': '@typescript-eslint/parser', + 'parserOptions': { + 'project': tsconfig, + 'tsconfigRootDir': '.', + 'ecmaVersion': 2020, + 'sourceType': 'module', + }, + 'extends': [ + 'plugin:@typescript-eslint/recommended', + 'plugin:jsdoc/recommended', + ], + 'rules': { + // TS rules + // Blockly uses namespaces to do declaration merging in some cases. + '@typescript-eslint/no-namespace': ['off'], + // Use the updated TypeScript-specific rule. + 'no-invalid-this': ['off'], + '@typescript-eslint/no-invalid-this': ['error'], + // Needs decision. 601 problems. + '@typescript-eslint/no-non-null-assertion': ['off'], + // Use TS-specific rule. + 'no-unused-vars': ['off'], + '@typescript-eslint/no-unused-vars': [ + 'warn', + { + 'argsIgnorePattern': '^_', + 'varsIgnorePattern': '^_', + }, + ], + 'func-call-spacing': ['off'], + '@typescript-eslint/func-call-spacing': ['warn'], + // Temporarily disable. 23 problems. + '@typescript-eslint/no-explicit-any': ['off'], + // Temporarily disable. 128 problems. + 'require-jsdoc': ['off'], + // Temporarily disable. 55 problems. + '@typescript-eslint/ban-types': ['off'], + // Temporarily disable. 33 problems. + '@typescript-eslint/no-empty-function': ['off'], + // Temporarily disable. 3 problems. + '@typescript-eslint/no-empty-interface': ['off'], + + // TsDoc rules (using JsDoc plugin) + // Disable built-in jsdoc verifier. + 'valid-jsdoc': ['off'], + // Don't require types in params and returns docs. + 'jsdoc/require-param-type': ['off'], + 'jsdoc/require-returns-type': ['off'], + // params and returns docs are optional. + 'jsdoc/require-param-description': ['off'], + 'jsdoc/require-returns': ['off'], + // Disable for now (breaks on `this` which is not really a param). + 'jsdoc/require-param': ['off'], + // Don't auto-add missing jsdoc. Only required on exported items. + 'jsdoc/require-jsdoc': [ + 'warn', + { + 'enableFixer': false, + 'publicOnly': true, + }, + ], + // Disable because of false alarms with Closure-supported tags. + // Re-enable after Closure is removed. + 'jsdoc/check-tag-names': ['off'], + // Re-enable after Closure is removed. There shouldn't even be + // types in the TsDoc. + // These are "types" because of Closure's @suppress {warningName} + 'jsdoc/no-undefined-types': ['off'], + 'jsdoc/valid-types': ['off'], + // Disabled due to not handling `this`. If re-enabled, + // checkDestructured option + // should be left as false. + 'jsdoc/check-param-names': ['off', {'checkDestructured': false}], + // Allow any text in the license tag. Other checks are not relevant. + 'jsdoc/check-values': ['off'], + }, + }; +} + +// NOTE: When this output is put directly in `module.exports`, the formatter +// does not align with the linter. +const eslintJSON = { + 'rules': rules, + 'env': { + 'es2020': true, + 'browser': true, + }, + 'globals': { + 'goog': true, + 'exports': true, + }, + 'extends': [ + 'eslint:recommended', + 'google', + ], + // TypeScript-specific config. Uses above rules plus these. + 'overrides': [ + buildTSOverride({ + files: ['./core/**/*.ts', './core/**/*.tsx'], + tsconfig: './tsconfig.json', + }), + buildTSOverride({ + files: [ + './tests/typescript/**/*.ts', + './tests/typescript/**/*.tsx', + ], + tsconfig: './tests/typescript/tsconfig.json', + }), + { + 'files': ['./.eslintrc.js'], + 'env': { + 'node': true, + }, + }, + ], +}; + +module.exports = eslintJSON; diff --git a/.eslintrc.json b/.eslintrc.json deleted file mode 100644 index eb5b9f339..000000000 --- a/.eslintrc.json +++ /dev/null @@ -1,165 +0,0 @@ -{ - "rules": { - "curly": ["error"], - "eol-last": ["error"], - "keyword-spacing": ["error"], - "linebreak-style": ["error", "unix"], - "max-len": [ - "error", - { - "code": 100, - "tabWidth": 4, - "ignoreStrings": true, - "ignoreRegExpLiterals": true, - "ignoreUrls": true - } - ], - "no-trailing-spaces": ["error", { "skipBlankLines": true }], - "no-unused-vars": [ - "warn", - { - "args": "after-used", - // Ignore vars starting with an underscore. - "varsIgnorePattern": "^_", - // Ignore arguments starting with an underscore. - "argsIgnorePattern": "^_" - } - ], - // Blockly uses for exporting symbols. no-self-assign added in eslint 5. - "no-self-assign": ["off"], - // Blockly uses single quotes except for JSON blobs, which must use double quotes. - "quotes": ["off"], - "semi": ["error", "always"], - // Blockly doesn't have space before function paren when defining functions. - "space-before-function-paren": ["error", "never"], - // Blockly doesn't have space before function paren when calling functions. - "func-call-spacing": ["error", "never"], - "space-infix-ops": ["error"], - // Blockly uses 'use strict' in files. - "strict": ["off"], - // Closure style allows redeclarations. - "no-redeclare": ["off"], - "valid-jsdoc": ["error"], - "no-console": ["off"], - "no-multi-spaces": ["error", { "ignoreEOLComments": true }], - "operator-linebreak": ["error", "after"], - "spaced-comment": ["error", "always", { - "block": { - "balanced": true - }, - "exceptions": ["*"] - }], - // Blockly uses prefixes for optional arguments and test-only functions. - "camelcase": ["error", { - "properties": "never", - "allow": ["^opt_", "^_opt_", "^testOnly_"] - }], - // Use clang-format for indentation by running `npm run format`. - "indent": ["off"], - // Blockly uses capital letters for some non-constructor namespaces. - // Keep them for legacy reasons. - "new-cap": ["off"], - // Mostly use default rules for brace style, but allow single-line blocks. - "brace-style": ["error", "1tbs", { "allowSingleLine": true }], - // Blockly uses objects as maps, but uses Object.create(null) to - // instantiate them. - "guard-for-in": ["off"], - "prefer-spread": ["off"], - "comma-dangle": ["error", { - "arrays": "always-multiline", - "objects": "always-multiline", - "imports": "always-multiline", - "exports": "always-multiline", - "functions": "ignore" - }] - }, - "env": { - "es2020": true, - "browser": true - }, - "globals": { - "goog": true, - "exports": true - }, - "extends": [ - "eslint:recommended", "google" - ], - // TypeScript-specific config. Uses above rules plus these. - "overrides": [{ - "files": ["**/*.ts", "**/*.tsx"], - "plugins": [ - "@typescript-eslint/eslint-plugin", - "jsdoc" - ], - "settings": { - "jsdoc": { - "mode": "typescript" - } - }, - "parser": "@typescript-eslint/parser", - "parserOptions": { - "project": "./tsconfig.json", - "tsconfigRootDir": ".", - "ecmaVersion": 2020, - "sourceType": "module" - }, - "extends": [ - "plugin:@typescript-eslint/recommended", - "plugin:jsdoc/recommended" - ], - "rules": { - // TS rules - // Blockly uses namespaces to do declaration merging in some cases. - "@typescript-eslint/no-namespace": ["off"], - // Use the updated TypeScript-specific rule. - "no-invalid-this": ["off"], - "@typescript-eslint/no-invalid-this": ["error"], - // Needs decision. 601 problems. - "@typescript-eslint/no-non-null-assertion": ["off"], - // Use TS-specific rule. - "no-unused-vars": ["off"], - "@typescript-eslint/no-unused-vars": ["warn", { - "argsIgnorePattern": "^_", - "varsIgnorePattern": "^_" - }], - "func-call-spacing": ["off"], - "@typescript-eslint/func-call-spacing": ["warn"], - // Temporarily disable. 23 problems. - "@typescript-eslint/no-explicit-any": ["off"], - // Temporarily disable. 128 problems. - "require-jsdoc": ["off"], - // Temporarily disable. 55 problems. - "@typescript-eslint/ban-types": ["off"], - // Temporarily disable. 33 problems. - "@typescript-eslint/no-empty-function": ["off"], - // Temporarily disable. 3 problems. - "@typescript-eslint/no-empty-interface": ["off"], - - // TsDoc rules (using JsDoc plugin) - // Disable built-in jsdoc verifier. - "valid-jsdoc": ["off"], - // Don't require types in params and returns docs. - "jsdoc/require-param-type": ["off"], - "jsdoc/require-returns-type": ["off"], - // params and returns docs are optional. - "jsdoc/require-param-description": ["off"], - "jsdoc/require-returns": ["off"], - // Disable for now (breaks on `this` which is not really a param). - "jsdoc/require-param": ["off"], - // Don't auto-add missing jsdoc. Only required on exported items. - "jsdoc/require-jsdoc": ["warn", {"enableFixer": false, "publicOnly": true}], - // Disable because of false alarms with Closure-supported tags. - // Re-enable after Closure is removed. - "jsdoc/check-tag-names": ["off"], - // Re-enable after Closure is removed. There shouldn't even be types in the TsDoc. - // These are "types" because of Closure's @suppress {warningName} - "jsdoc/no-undefined-types": ["off"], - "jsdoc/valid-types": ["off"], - // Disabled due to not handling `this`. If re-enabled, checkDestructured option - // should be left as false. - "jsdoc/check-param-names": ["off", {"checkDestructured": false}], - // Allow any text in the license tag. Other checks are not relevant. - "jsdoc/check-values": ["off"] - } - }] -} diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index eacbc2c88..72efb5fee 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -733,7 +733,8 @@ function cleanBuildDir() { function format() { return gulp.src([ 'core/**/*.js', 'core/**/*.ts', - 'blocks/**/*.js', 'blocks/**/*.ts' + 'blocks/**/*.js', 'blocks/**/*.ts', + '.eslintrc.js' ], {base: '.'}) .pipe(clangFormatter.format('file', clangFormat)) .pipe(gulp.dest('.')); diff --git a/scripts/gulpfiles/config.js b/scripts/gulpfiles/config.js index 5f3cad99d..674432dde 100644 --- a/scripts/gulpfiles/config.js +++ b/scripts/gulpfiles/config.js @@ -38,6 +38,9 @@ exports.TYPINGS_BUILD_DIR = path.join(exports.BUILD_DIR, 'declarations'); // Matches the value in tsconfig.json: outDir exports.TSC_OUTPUT_DIR = path.join(exports.BUILD_DIR, 'src'); +// Directory for files generated by compiling test code. +exports.TEST_TSC_OUTPUT_DIR = path.join(exports.BUILD_DIR, 'tests'); + // Directory in which to assemble (and from which to publish) the // blockly npm package. exports.RELEASE_DIR = 'dist'; diff --git a/scripts/gulpfiles/test_tasks.js b/scripts/gulpfiles/test_tasks.js index 4ba23b683..0354977bf 100644 --- a/scripts/gulpfiles/test_tasks.js +++ b/scripts/gulpfiles/test_tasks.js @@ -17,8 +17,7 @@ const path = require('path'); const {execSync} = require('child_process'); const rimraf = require('rimraf'); -const buildTasks = require('./build_tasks'); -const {BUILD_DIR, RELEASE_DIR} = require('./config'); +const {RELEASE_DIR, TEST_TSC_OUTPUT_DIR} = require('./config'); const {runMochaTestsInBrowser} = require('../../tests/mocha/webdriver.js'); const {runGeneratorsInBrowser} = require('../../tests/generators/webdriver.js'); @@ -371,6 +370,16 @@ function advancedCompileInBrowser() { return runTestTask('advanced_compile_in_browser', runCompileCheckInBrowser); } +/** + * Verify the built Blockly type definitions compile with the supported + * TypeScript examples included in `./tests/typescript`. + * @returns {Promise} Asynchronous result. + */ +function typeDefinitions() { + return runTestCommand('type_definitions', + `tsc -p ./tests/typescript/tsconfig.json -outDir ${TEST_TSC_OUTPUT_DIR}`); +} + // Run all tests in sequence. const tasks = [ eslint, @@ -381,6 +390,7 @@ const tasks = [ mocha, generators, node, + typeDefinitions, // Make sure these two are in series with each other advancedCompile, advancedCompileInBrowser diff --git a/tests/mocha/.eslintrc.json b/tests/mocha/.eslintrc.json index 99dda9b38..a35932d7b 100644 --- a/tests/mocha/.eslintrc.json +++ b/tests/mocha/.eslintrc.json @@ -14,7 +14,7 @@ "prefer-rest-params": ["off"], "no-invalid-this": ["off"] }, - "extends": "../../.eslintrc.json", + "extends": "../../.eslintrc.js", "parserOptions": { "sourceType": "module" } diff --git a/tests/node/.eslintrc.json b/tests/node/.eslintrc.json index 287b288a6..9252228ed 100644 --- a/tests/node/.eslintrc.json +++ b/tests/node/.eslintrc.json @@ -8,5 +8,5 @@ "console": true, "require": true }, - "extends": "../../.eslintrc.json" + "extends": "../../.eslintrc.js" } diff --git a/tests/typescript/README.md b/tests/typescript/README.md new file mode 100644 index 000000000..f5c28e3ab --- /dev/null +++ b/tests/typescript/README.md @@ -0,0 +1,4 @@ +# TypeScript Tests + +Tests pass if all files here compile. Add Blockly use case examples we'd like +to support, such as subclasses which extend from Blockly classes. diff --git a/tests/typescript/src/field/different_user_input.ts b/tests/typescript/src/field/different_user_input.ts new file mode 100644 index 000000000..f77094ea8 --- /dev/null +++ b/tests/typescript/src/field/different_user_input.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright 2022 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Test: Should allow a subclass of Field to have different setValue and + * constructor input type than the type that is stored. + */ + + +import {Field, FieldValidator, fieldRegistry} from 'blockly-test/core'; + +interface Cell { + cellId: string; +} + +interface CellGroup { + cells: Cell[]; +} + +type FieldMitosisValidator = FieldValidator; + +class FieldMitosis extends Field { + constructor(cell: Cell, validator: FieldMitosisValidator) { + super(Field.SKIP_SETUP); + + this.setValue(cell); + this.setValidator(validator); + } + + // Overwritten Field methods. + + protected doClassValidation_(newCell?: unknown): CellGroup | null { + if (!this.isCell(newCell)) return null; + + const cellGroup = this.getValue() ?? {cells: []}; + cellGroup.cells.push(newCell); + return cellGroup; + } + + // Example-specific methods. + + private isCell(maybeCell: unknown): maybeCell is Cell { + if (!maybeCell) return false; + const couldBeCell = maybeCell as { [key: string]: unknown }; + return 'cellId' in couldBeCell && typeof couldBeCell.cellId === 'string'; + } + + /** + * The cell divides, creating two new cells! + */ + doMitosis(): void { + const cellGroup = this.getValue(); + if (!cellGroup) return; + + const cells = cellGroup.cells.flatMap((cell) => { + const leftCell: Cell = {cellId: `${cell.cellId}-left`}; + const rightCell: Cell = {cellId: `${cell.cellId}-right`}; + return [leftCell, rightCell]; + }); + this.value_ = {cells}; + } +} + +fieldRegistry.register('field_mitosis', FieldMitosis); + +// Example use of the class. + +function cellValidator(cellGroup: CellGroup): CellGroup | undefined { + // The cell group is good! Use it as is. + if (cellGroup.cells.length > 0) return undefined; + + // Uh oh! No cells. + const emergencyCell: Cell = {cellId: 'emergency-cell'}; + return {cells: [emergencyCell]}; +} + +const cellField = new FieldMitosis({cellId: 'cell-A'}, cellValidator); +cellField.setValue({cellId: 'cell-B'}); +cellField.doMitosis(); diff --git a/tests/typescript/tsconfig.json b/tests/typescript/tsconfig.json new file mode 100644 index 000000000..162b9d56b --- /dev/null +++ b/tests/typescript/tsconfig.json @@ -0,0 +1,22 @@ +{ + "include": [ + "src/**/*", + ], + "compilerOptions": { + "allowJs": false, + "outDir": "dist", + "baseUrl": ".", + "paths": { + "blockly-test/*": ["../../dist/*"], + }, + + "declaration": false, + "declarationMap": false, + "sourceMap": false, + + "module": "ES2015", + "moduleResolution": "node", + "target": "ES2020", + "strict": true, + } +}