From b6ca05172265b2afe0c76c363c16ab1984ba976d Mon Sep 17 00:00:00 2001 From: Joshua Wink Date: Sat, 10 Oct 2026 14:54:06 -0500 Subject: [PATCH] feat(typescript): a linter for TypeScript pipeline code packages/codeupipe-lint-ts reads TypeScript source and reports: - a step with no name: a function, an object literal, a class that is declared in a function, a plain nested Pipeline (CUP-A001, A002, A003, A005, A008, A009); - a Filter or a Predicate that reads State (CUP-I003); - a Tap, a Predicate or a Hook that writes to the Payload (CUP-I004); - more than one primitive in a file, a file name that is not the class name in kebab-case, a class name with no role (CUP-I005, I006, I007). The linter is a codeupipe pipeline. It uses the typescript package to parse. A finding is turned off with a comment marker, --exclude, --disable, or the lint section of cup.config.json. CUP-I001 is not needed in TypeScript, because the data of a Payload is in a private field. CUP-I002 is not built. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 27 +++ README.md | 10 +- conformance/API.md | 2 +- packages/codeupipe-lint-ts/.gitignore | 2 + packages/codeupipe-lint-ts/LICENSE | 177 ++++++++++++++++++ packages/codeupipe-lint-ts/NOTICE | 4 + packages/codeupipe-lint-ts/README.md | 108 +++++++++++ packages/codeupipe-lint-ts/package-lock.json | 73 ++++++++ packages/codeupipe-lint-ts/package.json | 39 ++++ .../src/analyze-outside-writes-filter.ts | 73 ++++++++ .../src/analyze-primitive-names-filter.ts | 54 ++++++ .../src/analyze-primitives-per-file-filter.ts | 32 ++++ .../src/analyze-state-reads-filter.ts | 111 +++++++++++ .../src/analyze-steps-filter.ts | 45 +++++ .../src/apply-suppressions-filter.ts | 41 ++++ packages/codeupipe-lint-ts/src/cli.ts | 68 +++++++ .../src/collect-files-filter.ts | 53 ++++++ .../src/drop-disabled-filter.ts | 17 ++ packages/codeupipe-lint-ts/src/finding.ts | 19 ++ packages/codeupipe-lint-ts/src/fnmatch.ts | 28 +++ .../src/format-report-filter.ts | 29 +++ packages/codeupipe-lint-ts/src/index.ts | 5 + packages/codeupipe-lint-ts/src/lint-paths.ts | 13 ++ .../codeupipe-lint-ts/src/lint-pipeline.ts | 29 +++ .../src/parse-files-filter.ts | 45 +++++ .../codeupipe-lint-ts/src/primitive-kind.ts | 84 +++++++++ packages/codeupipe-lint-ts/src/read-config.ts | 43 +++++ packages/codeupipe-lint-ts/src/rules.ts | 68 +++++++ packages/codeupipe-lint-ts/src/source.ts | 10 + .../codeupipe-lint-ts/src/step-classifier.ts | 125 +++++++++++++ .../tests/anonymous-steps.test.ts | 126 +++++++++++++ packages/codeupipe-lint-ts/tests/cli.test.ts | 81 ++++++++ .../tests/invariants.test.ts | 127 +++++++++++++ packages/codeupipe-lint-ts/tests/lint-in.ts | 28 +++ .../codeupipe-lint-ts/tsconfig.build.json | 10 + packages/codeupipe-lint-ts/tsconfig.json | 19 ++ packages/codeupipe-ts/README.md | 3 +- 37 files changed, 1822 insertions(+), 6 deletions(-) create mode 100644 packages/codeupipe-lint-ts/.gitignore create mode 100644 packages/codeupipe-lint-ts/LICENSE create mode 100644 packages/codeupipe-lint-ts/NOTICE create mode 100644 packages/codeupipe-lint-ts/README.md create mode 100644 packages/codeupipe-lint-ts/package-lock.json create mode 100644 packages/codeupipe-lint-ts/package.json create mode 100644 packages/codeupipe-lint-ts/src/analyze-outside-writes-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/analyze-primitive-names-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/analyze-primitives-per-file-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/analyze-state-reads-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/analyze-steps-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/apply-suppressions-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/cli.ts create mode 100644 packages/codeupipe-lint-ts/src/collect-files-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/drop-disabled-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/finding.ts create mode 100644 packages/codeupipe-lint-ts/src/fnmatch.ts create mode 100644 packages/codeupipe-lint-ts/src/format-report-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/index.ts create mode 100644 packages/codeupipe-lint-ts/src/lint-paths.ts create mode 100644 packages/codeupipe-lint-ts/src/lint-pipeline.ts create mode 100644 packages/codeupipe-lint-ts/src/parse-files-filter.ts create mode 100644 packages/codeupipe-lint-ts/src/primitive-kind.ts create mode 100644 packages/codeupipe-lint-ts/src/read-config.ts create mode 100644 packages/codeupipe-lint-ts/src/rules.ts create mode 100644 packages/codeupipe-lint-ts/src/source.ts create mode 100644 packages/codeupipe-lint-ts/src/step-classifier.ts create mode 100644 packages/codeupipe-lint-ts/tests/anonymous-steps.test.ts create mode 100644 packages/codeupipe-lint-ts/tests/cli.test.ts create mode 100644 packages/codeupipe-lint-ts/tests/invariants.test.ts create mode 100644 packages/codeupipe-lint-ts/tests/lint-in.ts create mode 100644 packages/codeupipe-lint-ts/tsconfig.build.json create mode 100644 packages/codeupipe-lint-ts/tsconfig.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57ad35c..540d642 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,6 +66,33 @@ jobs: working-directory: . run: python conformance/run.py typescript:run + typescript-lint: + name: TypeScript linter + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-node@v5 + with: + node-version: "24" + - name: Install the core package + working-directory: packages/codeupipe-ts + run: npm ci + - name: Install + working-directory: packages/codeupipe-lint-ts + run: npm ci + - name: Type check + working-directory: packages/codeupipe-lint-ts + run: npm run typecheck + - name: Tests + working-directory: packages/codeupipe-lint-ts + run: npm test + - name: Build + working-directory: packages/codeupipe-lint-ts + run: npm run build + - name: The linter on the TypeScript code of this repository + working-directory: packages/codeupipe-lint-ts + run: npm run lint + export: name: Export against dynamic run runs-on: ubuntu-latest diff --git a/README.md b/README.md index 43f8f78..b5c83e7 100644 --- a/README.md +++ b/README.md @@ -29,13 +29,14 @@ If you know Unix, these are close to things you know. | Hook | A git hook: code that runs before and after a step | | State | `$?` and `PIPESTATUS` | -This repository has three packages. +This repository has four packages. | Package | Contents | Reference | |---|---|---| | `codeupipe` | The eight primitives. An export command that writes a pipeline as plain Python with no import of codeupipe. A config file reader. No dependencies | [conformance/API.md](conformance/API.md), [docs/static-export.md](docs/static-export.md), [docs/config.md](docs/config.md) | | `codeupipe-lint` | Two linters for pipeline code. Depends on `codeupipe` only | [packages/codeupipe-lint/README.md](packages/codeupipe-lint/README.md) | -| `codeupipe` for TypeScript | The eight primitives. No dependencies. No export and no linter | [packages/codeupipe-ts/README.md](packages/codeupipe-ts/README.md) | +| `codeupipe` for TypeScript | The eight primitives. No dependencies | [packages/codeupipe-ts/README.md](packages/codeupipe-ts/README.md) | +| `codeupipe-lint` for TypeScript | A linter for pipeline code in TypeScript | [packages/codeupipe-lint-ts/README.md](packages/codeupipe-lint-ts/README.md) | ## Install @@ -166,6 +167,7 @@ An editable install needs pip 21.3 or later. | `packages/codeupipe/` | The `codeupipe` package: `core/`, `lower/` (the export), `config.py` | | `packages/codeupipe-lint/` | The `codeupipe-lint` package: `anonymous_steps/`, `invariants/` | | `packages/codeupipe-ts/` | The TypeScript runtime, with its tests and its example | +| `packages/codeupipe-lint-ts/` | The TypeScript linter, with its tests | | `conformance/` | The contract ([API.md](conformance/API.md)), the cases, the runners, the benchmark | | `tests/` | The tests for the two Python packages | | `examples/pricing/` | The example, with its test in `tests/examples/` | @@ -178,8 +180,8 @@ The root `pyproject.toml` is not a package. It holds the test settings. The contract and the conformance cases do not depend on a language. Python is the first runtime and TypeScript is the second -([packages/codeupipe-ts/](packages/codeupipe-ts/)). The export and the -linters are for Python only. A runtime in another language must implement +([packages/codeupipe-ts/](packages/codeupipe-ts/)), with its own linter. The +export is for Python only. A runtime in another language must implement the contract and add a conformance runner. ## License diff --git a/conformance/API.md b/conformance/API.md index 876fec8..1059875 100644 --- a/conformance/API.md +++ b/conformance/API.md @@ -214,7 +214,7 @@ map and fails the comparison. | Absent | `None` | `null` | | Label of a step | `type(step).__name__` | `step.constructor.name` | | Export | `codeupipe.lower` | none | -| Linter | `codeupipe-lint` | none | +| Linter | `packages/codeupipe-lint` | `packages/codeupipe-lint-ts` | Notes on TypeScript: diff --git a/packages/codeupipe-lint-ts/.gitignore b/packages/codeupipe-lint-ts/.gitignore new file mode 100644 index 0000000..b947077 --- /dev/null +++ b/packages/codeupipe-lint-ts/.gitignore @@ -0,0 +1,2 @@ +node_modules/ +dist/ diff --git a/packages/codeupipe-lint-ts/LICENSE b/packages/codeupipe-lint-ts/LICENSE new file mode 100644 index 0000000..f433b1a --- /dev/null +++ b/packages/codeupipe-lint-ts/LICENSE @@ -0,0 +1,177 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS diff --git a/packages/codeupipe-lint-ts/NOTICE b/packages/codeupipe-lint-ts/NOTICE new file mode 100644 index 0000000..fd0d109 --- /dev/null +++ b/packages/codeupipe-lint-ts/NOTICE @@ -0,0 +1,4 @@ +codeupipe +Copyright 2026 Orchestrate LLC + +Licensed under the Apache License, Version 2.0. See LICENSE. diff --git a/packages/codeupipe-lint-ts/README.md b/packages/codeupipe-lint-ts/README.md new file mode 100644 index 0000000..b5a98a6 --- /dev/null +++ b/packages/codeupipe-lint-ts/README.md @@ -0,0 +1,108 @@ +# codeupipe-lint for TypeScript + +A linter for TypeScript code that uses codeupipe. It reads the source and +does not run it. It is not on npm yet. + +The linter is a codeupipe pipeline: [src/lint-pipeline.ts](src/lint-pipeline.ts). +It depends on `codeupipe` and on the `typescript` package, which it uses to +parse the files. + +```bash +codeupipe-lint src # files or directories +codeupipe-lint --exclude '*/generated/*' src +codeupipe-lint --disable CUP-I005 src +codeupipe-lint --rules # list the rule ids +codeupipe-lint # paths come from cup.config.json +``` + +Each finding is one line, and the message names the fix: + +``` +path:line:col CUP-A001 lambda-step Filter is a function. Define it at module level as a named class (...) +``` + +Exit code: **0** no findings, **1** findings, **2** a path could not be +read or the arguments cannot be used. + +From TypeScript: `lintPaths(paths, { exclude, disable })` returns the result +Payload (`findings`, `suppressed`, `errors`, `report`, `exitCode`). + +## Rules + +The ids are those of the Python linter, where it has the rule. + +| id | slug | what | +|---|---|---| +| CUP-I000 | parse-error | the file has a syntax error | +| CUP-A001 | lambda-step | an arrow function or a function expression as a step | +| CUP-A002 | local-function-step | a named function as a Filter or a Tap | +| CUP-A003 | local-type-step | an instance of a class that is declared in a function, or of a class with no name | +| CUP-A005 | unnamed-pipeline-step | a plain `new Pipeline()` nested as a step | +| CUP-A008 | function-predicate | a named function as a Valve predicate | +| CUP-A009 | object-literal-step | an object literal as a step. TypeScript only | +| CUP-I003 | state-read-in-step | a Filter (`call`) or a Predicate (`test`) reads the State of a pipeline | +| CUP-I004 | write-outside-filter | a Tap, a Predicate or a Hook writes to the Payload | +| CUP-I005 | one-primitive-per-file | a second primitive class at the top level of a file | +| CUP-I006 | primitive-file-name | the file name is not the class name in kebab-case | +| CUP-I007 | primitive-role-suffix | the class name does not end with its role | + +A step is the argument of `addFilter`, `addTap` or `addPipeline` (on any +receiver), and the two arguments of `addValve(predicate, filter)`. + +A class is a primitive by its shape: `extends Pipeline`, `extends Hook`, +`implements Filter`, `implements Tap`, `implements Predicate`, or a method +`call`, `observe` or `test` with exactly one required parameter. + +## Rules of the Python linter that are not here + +- **CUP-I001** (a change to the Payload behind its API). The data of a + Payload is in a private field (`#data`), so the language refuses this. +- **CUP-I002** (a change to a value that Payloads share). Not built yet. + +## Turn a finding off + +- **One line.** A comment on the line of the finding, on the line of the + `add...` call around it, or alone on the line directly above: + `// cup: allow-anonymous` for a CUP-A finding; + `// cup: allow-state-read`, `allow-outside-write`, `allow-shared-file` or + `allow-name` for a CUP-I finding. Any text can follow the marker. +- **Files.** `--exclude GLOB`, or `lint.exclude` in the config file. In a + glob, `*` is any text, a `/` too. +- **A rule, for the project.** `--disable RULE`, or `lint.disable` in the + config file. A rule id that does not exist is an error. + +## Config file + +`cup.config.json`, in the directory that the command runs from. The linter +reads the `lint` section and no other. + +```json +{"lint": {"paths": ["src"], "exclude": ["*/generated/*"], "disable": ["CUP-I005"]}} +``` + +The file is used only when no paths are given on the command line. A key in +`lint` that the linter does not read is an error. + +## Limits + +- A name is followed to its declaration in the same file when that is a + `const`, a function or a class. A `let`, a parameter, an import and the + result of a call are not followed, so a function that returns an anonymous + step is not seen. +- The linter reads syntax only. It does not use the types of the program. +- Test files (`*.test.ts`, `*.spec.ts`, and files under `tests/`) are not + checked by CUP-I005, CUP-I006 and CUP-I007. + +## Development + +```bash +npm install # links ../codeupipe-ts and installs the TypeScript compiler +npm test # builds the core package first, then runs the tests +npm run typecheck +npm run build # JavaScript and type files in dist/ +npm run lint # the linter on its own source, the core package and the conformance runner +``` + +## License + +Apache License 2.0. diff --git a/packages/codeupipe-lint-ts/package-lock.json b/packages/codeupipe-lint-ts/package-lock.json new file mode 100644 index 0000000..c013f6c --- /dev/null +++ b/packages/codeupipe-lint-ts/package-lock.json @@ -0,0 +1,73 @@ +{ + "name": "codeupipe-lint", + "version": "0.1.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "codeupipe-lint", + "version": "0.1.0", + "license": "Apache-2.0", + "dependencies": { + "codeupipe": "file:../codeupipe-ts", + "typescript": "^5.9.0" + }, + "bin": { + "codeupipe-lint": "dist/cli.js" + }, + "devDependencies": { + "@types/node": "^24.0.0" + }, + "engines": { + "node": ">=22.18" + } + }, + "../codeupipe-ts": { + "name": "codeupipe", + "version": "0.1.0", + "license": "Apache-2.0", + "devDependencies": { + "@types/node": "^24.0.0", + "esbuild": "^0.28.2", + "typescript": "^5.9.0" + }, + "engines": { + "node": ">=22.18" + } + }, + "node_modules/@types/node": { + "version": "24.19.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.19.2.tgz", + "integrity": "sha512-93d49wCWJkCyN87MxLpNN6KXPCwFw7bVUg79mGUV1OS40JGWwVCr8n61DXCSqz7CV6/VfPkb7xec7uP2M+QHWQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": ">=7.24.0 <7.24.7" + } + }, + "node_modules/codeupipe": { + "resolved": "../codeupipe-ts", + "link": true + }, + "node_modules/typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + }, + "node_modules/undici-types": { + "version": "7.24.6", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.24.6.tgz", + "integrity": "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg==", + "dev": true, + "license": "MIT" + } + } +} diff --git a/packages/codeupipe-lint-ts/package.json b/packages/codeupipe-lint-ts/package.json new file mode 100644 index 0000000..6495f59 --- /dev/null +++ b/packages/codeupipe-lint-ts/package.json @@ -0,0 +1,39 @@ +{ + "name": "codeupipe-lint", + "version": "0.1.0", + "private": true, + "description": "A linter for TypeScript code that uses codeupipe.", + "license": "Apache-2.0", + "repository": { + "type": "git", + "url": "git+https://github.com/orchestrate-solutions/codeupipe.git", + "directory": "packages/codeupipe-lint-ts" + }, + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "bin": { + "codeupipe-lint": "./dist/cli.js" + }, + "files": ["dist", "LICENSE", "NOTICE", "README.md"], + "engines": { + "node": ">=22.18" + }, + "scripts": { + "core": "npm --prefix ../codeupipe-ts run build", + "pretest": "npm run core", + "test": "node --test \"tests/*.test.ts\"", + "pretypecheck": "npm run core", + "typecheck": "tsc --noEmit", + "prebuild": "npm run core", + "build": "tsc -p tsconfig.build.json", + "lint": "node src/cli.ts src ../codeupipe-ts/src ../codeupipe-ts/examples ../../conformance/runners/typescript" + }, + "dependencies": { + "codeupipe": "file:../codeupipe-ts", + "typescript": "^5.9.0" + }, + "devDependencies": { + "@types/node": "^24.0.0" + } +} diff --git a/packages/codeupipe-lint-ts/src/analyze-outside-writes-filter.ts b/packages/codeupipe-lint-ts/src/analyze-outside-writes-filter.ts new file mode 100644 index 0000000..8ca3c9b --- /dev/null +++ b/packages/codeupipe-lint-ts/src/analyze-outside-writes-filter.ts @@ -0,0 +1,73 @@ +import type { Filter, Payload } from "codeupipe"; +import ts from "typescript"; + +import type { Finding } from "./finding.ts"; +import { methodsOf } from "./primitive-kind.ts"; +import { WRITE_OUTSIDE_FILTER, makeFinding } from "./rules.ts"; +import type { Source } from "./source.ts"; + +const READER_METHODS = ["observe", "test", "before", "after", "onError"]; +const WRITES = new Set(["insert", "remove", "merge", "withTrace"]); + +/** The parameter that holds the Payload: the one named `payload`, else the last one. */ +function payloadParameter(method: ts.MethodDeclaration): string | undefined { + const names = method.parameters.map((parameter) => parameter.name.getText()); + return names.includes("payload") ? "payload" : names[names.length - 1]; +} + +/** + * CUP-I004: a write to the Payload from a method that only reads it. + * + * Only a Filter writes. A Tap (`observe`), a Predicate (`test`) and a Hook + * (`before`, `after`, `onError`) read. The runtime does not check, so this + * rule is the guard. Flagged: `insert`, `remove`, `merge` or `withTrace` on + * the Payload parameter of such a method; a chain is one finding. Not + * flagged: a write to `payload.copy()`, and the same method names on any + * other object. + * + * Input keys: `sources`, `findings`. + * Output keys: `findings`, with CUP-I004 findings appended. + */ +export class AnalyzeOutsideWritesFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + for (const source of payload.get("sources") ?? []) { + const inMethod = (name: string, method: ts.MethodDeclaration) => { + const parameter = payloadParameter(method); + if (parameter === undefined || method.body === undefined) return; + const writes = (node: ts.Node): node is ts.CallExpression => { + if (!ts.isCallExpression(node) || !ts.isPropertyAccessExpression(node.expression) + || !WRITES.has(node.expression.name.text)) { + return false; + } + const owner = node.expression.expression; + return ts.isIdentifier(owner) ? owner.text === parameter : writes(owner); + }; + const visit = (node: ts.Node) => { + if (ts.isFunctionLike(node) || ts.isClassLike(node)) return; // its own scope + if (writes(node)) { + const write = (node.expression as ts.PropertyAccessExpression).name.text; + findings.push(makeFinding(source, node, WRITE_OUTSIDE_FILTER, + `\`${name}\` writes to the Payload (\`${parameter}.${write}\`)`)); + node.arguments.forEach(visit); // the chain is one finding; its arguments are still checked + return; + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(method.body, visit); + }; + const visit = (node: ts.Node) => { + if (ts.isClassLike(node)) { + const methods = methodsOf(node); + for (const name of READER_METHODS) { + const method = methods.get(name); + if (method !== undefined) inMethod(name, method); + } + } + ts.forEachChild(node, visit); + }; + visit(source.tree); + } + return payload.insert("findings", findings); + } +} diff --git a/packages/codeupipe-lint-ts/src/analyze-primitive-names-filter.ts b/packages/codeupipe-lint-ts/src/analyze-primitive-names-filter.ts new file mode 100644 index 0000000..fc57791 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/analyze-primitive-names-filter.ts @@ -0,0 +1,54 @@ +import { basename } from "node:path"; + +import type { Filter, Payload } from "codeupipe"; + +import type { Finding } from "./finding.ts"; +import { isTestFile, topLevelPrimitives } from "./primitive-kind.ts"; +import { PRIMITIVE_FILE_NAME, PRIMITIVE_ROLE_SUFFIX, makeFinding } from "./rules.ts"; +import type { Source } from "./source.ts"; + +/** `ParseHTMLFilter` gives `parse-html-filter`. */ +export function kebabCase(className: string): string { + return className.replace(/(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])/g, "-").toLowerCase(); +} + +/** + * CUP-I007: the class name of a primitive ends with its role + * (`DiscountFilter`, `AuditTap`, `PrintStepsHook`, `AtLeastPredicate`, + * `PricingPipeline`). + * + * CUP-I006: the file name is the class name in kebab-case + * (`discount-filter.ts`). + * + * Checked: primitive classes at the top level of a file. Not checked: plain + * classes, and test files. + * + * Input keys: `sources`, `findings`. + * Output keys: `findings`, with CUP-I006 and CUP-I007 findings appended. + */ +export class AnalyzePrimitiveNamesFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + for (const source of payload.get("sources") ?? []) { + if (isTestFile(source.path)) continue; + const file = basename(source.path); + const dot = file.indexOf("."); + const stem = dot === -1 ? file : file.slice(0, dot); + const extension = dot === -1 ? "" : file.slice(dot); + for (const { node, name, kind } of topLevelPrimitives(source.tree)) { + const at = node.name ?? node; + let wanted = name; + if (!name.endsWith(kind)) { + wanted = name + kind; + findings.push(makeFinding(source, at, PRIMITIVE_ROLE_SUFFIX, + `${kind} \`${name}\` does not say what it is. Name it \`${wanted}\``)); + } + if (stem !== kebabCase(wanted)) { + findings.push(makeFinding(source, at, PRIMITIVE_FILE_NAME, + `\`${wanted}\` is in \`${file}\`. Put it in \`${kebabCase(wanted)}${extension}\``)); + } + } + } + return payload.insert("findings", findings); + } +} diff --git a/packages/codeupipe-lint-ts/src/analyze-primitives-per-file-filter.ts b/packages/codeupipe-lint-ts/src/analyze-primitives-per-file-filter.ts new file mode 100644 index 0000000..bdbba76 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/analyze-primitives-per-file-filter.ts @@ -0,0 +1,32 @@ +import type { Filter, Payload } from "codeupipe"; + +import type { Finding } from "./finding.ts"; +import { isTestFile, topLevelPrimitives } from "./primitive-kind.ts"; +import { ONE_PRIMITIVE_PER_FILE, makeFinding } from "./rules.ts"; +import type { Source } from "./source.ts"; + +/** + * CUP-I005: one primitive in each file. + * + * Flagged: each primitive class at the top level of a file after the first. + * Not flagged: plain classes and functions beside the primitive, classes in + * a function, and test files. + * + * Input keys: `sources`, `findings`. + * Output keys: `findings`, with CUP-I005 findings appended. + */ +export class AnalyzePrimitivesPerFileFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + for (const source of payload.get("sources") ?? []) { + if (isTestFile(source.path)) continue; + const [first, ...rest] = topLevelPrimitives(source.tree); + if (first === undefined) continue; + for (const other of rest) { + findings.push(makeFinding(source, other.node.name ?? other.node, ONE_PRIMITIVE_PER_FILE, + `${other.kind} \`${other.name}\` shares this file with ${first.kind} \`${first.name}\``)); + } + } + return payload.insert("findings", findings); + } +} diff --git a/packages/codeupipe-lint-ts/src/analyze-state-reads-filter.ts b/packages/codeupipe-lint-ts/src/analyze-state-reads-filter.ts new file mode 100644 index 0000000..230b179 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/analyze-state-reads-filter.ts @@ -0,0 +1,111 @@ +import type { Filter, Payload } from "codeupipe"; +import ts from "typescript"; + +import type { Finding } from "./finding.ts"; +import { methodsOf } from "./primitive-kind.ts"; +import { STATE_READ, makeFinding } from "./rules.ts"; +import type { Source } from "./source.ts"; + +const STEP_METHODS = ["call", "test"]; +const STATE_MEMBERS = new Set(["executed", "skipped", "errors", "timings", "markExecuted", "markSkipped", + "recordTiming", "recordError"]); + +function isThis(node: ts.Node): boolean { + return node.kind === ts.SyntaxKind.ThisKeyword; +} + +function isStateType(type: ts.TypeNode | undefined): boolean { + return type !== undefined && ts.isTypeReferenceNode(type) && type.typeName.getText().split(".").pop() === "State"; +} + +function namesAPipeline(node: ts.Expression): boolean { + const name = ts.isIdentifier(node) ? node.text : ts.isPropertyAccessExpression(node) ? node.name.text : ""; + return name.toLowerCase().includes("pipeline"); +} + +/** The properties of a class that hold a State: by their type, or by what the constructor gives them. */ +function stateHandles(node: ts.ClassLikeDeclaration): Set { + const handles = new Set(); + for (const member of node.members) { + if (ts.isPropertyDeclaration(member) && ts.isIdentifier(member.name)) { + const start = member.initializer; + if (isStateType(member.type) || (start !== undefined && ts.isNewExpression(start) && start.expression.getText() === "State")) { + handles.add(member.name.text); + } + } + if (!ts.isConstructorDeclaration(member) || member.body === undefined) continue; + const stateParameters = new Set(member.parameters.filter((p) => isStateType(p.type)).map((p) => p.name.getText())); + const visit = (inner: ts.Node) => { + if (ts.isBinaryExpression(inner) && inner.operatorToken.kind === ts.SyntaxKind.EqualsToken + && ts.isPropertyAccessExpression(inner.left) && isThis(inner.left.expression)) { + const value = inner.right; + const isState = (ts.isIdentifier(value) && stateParameters.has(value.text)) + || (ts.isPropertyAccessExpression(value) && value.name.text === "state") + || (ts.isNewExpression(value) && value.expression.getText() === "State"); + if (isState) handles.add(inner.left.name.text); + } + ts.forEachChild(inner, visit); + }; + visit(member.body); + } + return handles; +} + +/** + * CUP-I003: a Filter (`call`) or a Predicate (`test`) that reads the State + * of a pipeline. + * + * Flagged: `.state.` where the member is one of State's own, + * `.state` when the name says it is a pipeline, and a property + * that holds a State. Not flagged: `this.state` (the step's own property), a + * Payload key named "state", and anything in a Tap or a Hook. + * + * Input keys: `sources`, `findings`. + * Output keys: `findings`, with CUP-I003 findings appended. + */ +export class AnalyzeStateReadsFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + for (const source of payload.get("sources") ?? []) { + const inClass = (node: ts.ClassLikeDeclaration) => { + const handles = stateHandles(node); + const methods = methodsOf(node); + for (const name of STEP_METHODS) { + const body = methods.get(name)?.body; + if (body !== undefined) inBody(body, handles); + } + }; + const reachesState = (node: ts.PropertyAccessExpression, handles: Set): boolean => { + let outer: string | undefined; + let at: ts.Expression = node; + while (ts.isPropertyAccessExpression(at)) { + if (isThis(at.expression) && handles.has(at.name.text)) return true; + if (at.name.text === "state" && !isThis(at.expression) + && ((outer !== undefined && STATE_MEMBERS.has(outer)) || namesAPipeline(at.expression))) { + return true; + } + outer = at.name.text; + at = at.expression; + } + return false; + }; + const inBody = (body: ts.Node, handles: Set) => { + const visit = (node: ts.Node) => { + if (ts.isFunctionLike(node) || ts.isClassLike(node)) return; // its own scope + if (ts.isPropertyAccessExpression(node) && reachesState(node, handles)) { + findings.push(makeFinding(source, node, STATE_READ, `Step reads pipeline State (\`${node.getText()}\`)`)); + return; + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(body, visit); + }; + const visit = (node: ts.Node) => { + if (ts.isClassLike(node)) inClass(node); + ts.forEachChild(node, visit); + }; + visit(source.tree); + } + return payload.insert("findings", findings); + } +} diff --git a/packages/codeupipe-lint-ts/src/analyze-steps-filter.ts b/packages/codeupipe-lint-ts/src/analyze-steps-filter.ts new file mode 100644 index 0000000..66ec5a4 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/analyze-steps-filter.ts @@ -0,0 +1,45 @@ +import type { Filter, Payload } from "codeupipe"; +import ts from "typescript"; + +import type { Finding } from "./finding.ts"; +import { makeFinding } from "./rules.ts"; +import type { Source } from "./source.ts"; +import { classifyStep } from "./step-classifier.ts"; +import type { Role } from "./step-classifier.ts"; + +const STEP_METHODS: Record = { addFilter: "Filter", addTap: "Tap", addPipeline: "Pipeline" }; +const VALVE_ROLES: Role[] = ["Valve predicate", "Valve filter"]; + +/** + * CUP-A001, A002, A003, A005, A008, A009: a step that has no name. + * + * A step is the argument of `addFilter`, `addTap` or `addPipeline` (on any + * receiver), or one of the two arguments of `addValve(predicate, filter)`. + * + * Input keys: `sources`, `findings`. + * Output keys: `findings`, with the new findings appended. + */ +export class AnalyzeStepsFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + for (const source of payload.get("sources") ?? []) { + const visit = (node: ts.Node) => { + if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression)) { + const method = node.expression.name.text; + const roles = method === "addValve" ? VALVE_ROLES + : STEP_METHODS[method] !== undefined ? [STEP_METHODS[method]] : []; + roles.forEach((role, index) => { + const argument = node.arguments[index]; + const verdict = argument === undefined ? undefined : classifyStep(argument, role); + if (argument !== undefined && verdict !== undefined) { + findings.push(makeFinding(source, argument, verdict.rule, `${role} ${verdict.what}`, node)); + } + }); + } + ts.forEachChild(node, visit); + }; + visit(source.tree); + } + return payload.insert("findings", findings); + } +} diff --git a/packages/codeupipe-lint-ts/src/apply-suppressions-filter.ts b/packages/codeupipe-lint-ts/src/apply-suppressions-filter.ts new file mode 100644 index 0000000..fc00328 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/apply-suppressions-filter.ts @@ -0,0 +1,41 @@ +import type { Filter, Payload } from "codeupipe"; + +import type { Finding } from "./finding.ts"; +import type { Source } from "./source.ts"; + +const ANONYMOUS = /cup:\s*allow-anonymous\b/; +const INVARIANT = /cup:\s*allow-(state-read|outside-write|shared-file|name)\b/; +const COMMENT_ONLY = /^\s*(\/\/|\/\*|\*)/; + +/** + * Drops the findings that a comment allows. + * + * `// cup: allow-anonymous` allows a CUP-A finding. `// cup: allow-state-read`, + * `allow-outside-write`, `allow-shared-file` and `allow-name` allow a CUP-I + * finding. The comment is on the line of the finding, on the line of the + * `add...` call around it, or alone on the line directly above. Any text can + * follow the marker. + * + * Input keys: `findings`, `sources`. + * Output keys: `findings` (kept), `suppressed` (dropped). + */ +export class ApplySuppressionsFilter implements Filter { + call(payload: Payload): Payload { + const lines = new Map(); + for (const source of payload.get("sources") ?? []) { + lines.set(source.path, source.text.split(/\r?\n/)); + } + const kept: Finding[] = []; + const suppressed: Finding[] = []; + for (const finding of payload.get("findings") ?? []) { + const marker = finding.rule.startsWith("CUP-A") ? ANONYMOUS : INVARIANT; + const text = lines.get(finding.path) ?? []; + const line = (number: number | undefined) => (number === undefined ? "" : text[number - 1] ?? ""); + const above = line(finding.line - 1); + const allowed = marker.test(line(finding.line)) || marker.test(line(finding.callLine)) + || (COMMENT_ONLY.test(above) && marker.test(above)); + (allowed ? suppressed : kept).push(finding); + } + return payload.insert("findings", kept).insert("suppressed", suppressed); + } +} diff --git a/packages/codeupipe-lint-ts/src/cli.ts b/packages/codeupipe-lint-ts/src/cli.ts new file mode 100644 index 0000000..6b8efb4 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/cli.ts @@ -0,0 +1,68 @@ +#!/usr/bin/env node +/** + * codeupipe-lint [--exclude GLOB] [--disable RULE] [--config FILE] [--rules] [paths...] + * + * With no paths, they come from cup.config.json. Exit code: 0 no findings, + * 1 findings, 2 a path could not be read or the arguments cannot be used. + */ +import { parseArgs } from "node:util"; + +import { lintPaths } from "./lint-paths.ts"; +import { CONFIG_NAME, readLintConfig } from "./read-config.ts"; +import { RULES } from "./rules.ts"; + +async function main(argv: string[]): Promise { + let args; + try { + args = parseArgs({ + args: argv, allowPositionals: true, + options: { + exclude: { type: "string", multiple: true }, + disable: { type: "string", multiple: true }, + config: { type: "string" }, + rules: { type: "boolean" }, + }, + }); + } catch (error) { + console.error(`error: ${error instanceof Error ? error.message : String(error)}`); + return 2; + } + if (args.values.rules) { + for (const rule of Object.keys(RULES).sort()) console.log(`${rule} ${RULES[rule]}`); + return 0; + } + + let paths = args.positionals; + let exclude = args.values.exclude; + let disable = args.values.disable; + if (paths.length === 0) { + // The file stands in for the arguments only when no paths are given: a + // path that someone names is checked, whatever the file excludes. + try { + const lint = readLintConfig(args.values.config) ?? {}; + paths = lint.paths ?? []; + exclude = exclude ?? lint.exclude; + disable = disable ?? lint.disable; + } catch (error) { + console.error(`error: ${error instanceof Error ? error.message : String(error)}`); + return 2; + } + } + if (paths.length === 0) { + console.error(`error: give at least one path, or list them under lint.paths in ${CONFIG_NAME}`); + return 2; + } + const unknown = (disable ?? []).filter((rule) => RULES[rule] === undefined); + if (unknown.length > 0) { + console.error(`error: no such rule: ${unknown.join(", ")} (the rules are listed by --rules)`); + return 2; + } + + const out = await lintPaths(paths, { exclude, disable }); + console.log(out.get("report")); + const errorReport = out.get("errorReport"); + if (errorReport) console.error(errorReport); + return out.get("exitCode") ?? 2; +} + +process.exitCode = await main(process.argv.slice(2)); diff --git a/packages/codeupipe-lint-ts/src/collect-files-filter.ts b/packages/codeupipe-lint-ts/src/collect-files-filter.ts new file mode 100644 index 0000000..fcecebd --- /dev/null +++ b/packages/codeupipe-lint-ts/src/collect-files-filter.ts @@ -0,0 +1,53 @@ +import { readdirSync, statSync } from "node:fs"; +import { join } from "node:path"; + +import type { Filter, Payload } from "codeupipe"; + +import { fnmatch } from "./fnmatch.ts"; + +const EXTENSIONS = [".ts", ".tsx", ".mts", ".cts"]; +const SKIP_DIRS = new Set([".git", "node_modules", "build", "dist", "coverage"]); + +function isSource(path: string): boolean { + return EXTENSIONS.some((extension) => path.endsWith(extension)) && !/\.d\.[cm]?ts$/.test(path); +} + +/** + * The TypeScript source files under the given paths. + * + * Input keys: `paths` (files or directories), optional `exclude` (globs + * matched against the path of each file). + * Output keys: `files` (paths), `errors` (appended). + * + * Directories are walked. Version control, dependency and build-output + * directories are not read, and nor are `.d.ts` files. + */ +export class CollectFilesFilter implements Filter { + call(payload: Payload): Payload { + const exclude = payload.get("exclude") ?? []; + const errors = [...(payload.get("errors") ?? [])]; + const files: string[] = []; + + const take = (path: string) => { + if (isSource(path) && !exclude.some((pattern) => fnmatch(path, pattern))) files.push(path); + }; + const walk = (directory: string) => { + for (const entry of readdirSync(directory, { withFileTypes: true }).sort((a, b) => (a.name < b.name ? -1 : 1))) { + const path = join(directory, entry.name); + if (entry.isDirectory()) { + if (!SKIP_DIRS.has(entry.name)) walk(path); + } else if (entry.isFile()) { + take(path); + } + } + }; + + for (const root of payload.get("paths") ?? []) { + const stat = statSync(root, { throwIfNoEntry: false }); + if (stat === undefined) errors.push(`${root}: no such file or directory`); + else if (stat.isDirectory()) walk(root); + else take(root); + } + return payload.insert("files", files).insert("errors", errors); + } +} diff --git a/packages/codeupipe-lint-ts/src/drop-disabled-filter.ts b/packages/codeupipe-lint-ts/src/drop-disabled-filter.ts new file mode 100644 index 0000000..b99f6ce --- /dev/null +++ b/packages/codeupipe-lint-ts/src/drop-disabled-filter.ts @@ -0,0 +1,17 @@ +import type { Filter, Payload } from "codeupipe"; + +import type { Finding } from "./finding.ts"; + +/** + * Drops the findings of the rules that the project switched off. + * + * Input keys: `findings`, optional `disable` (rule ids). + * Output keys: `findings`. + */ +export class DropDisabledFilter implements Filter { + call(payload: Payload): Payload { + const disabled = new Set(payload.get("disable") ?? []); + const findings = payload.get("findings") ?? []; + return payload.insert("findings", findings.filter((finding) => !disabled.has(finding.rule))); + } +} diff --git a/packages/codeupipe-lint-ts/src/finding.ts b/packages/codeupipe-lint-ts/src/finding.ts new file mode 100644 index 0000000..d4e3088 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/finding.ts @@ -0,0 +1,19 @@ +/** + * One finding, at `path:line:col` (line and column start at 1). + * + * `callLine` is the line of the `add...` call around the finding, when it is + * a different line: a comment that suppresses can be on that line too. + */ +export type Finding = { + path: string; + line: number; + col: number; + rule: string; + slug: string; + message: string; + callLine?: number; +}; + +export function formatFinding(finding: Finding): string { + return `${finding.path}:${finding.line}:${finding.col} ${finding.rule} ${finding.slug} ${finding.message}`; +} diff --git a/packages/codeupipe-lint-ts/src/fnmatch.ts b/packages/codeupipe-lint-ts/src/fnmatch.ts new file mode 100644 index 0000000..5f3c291 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/fnmatch.ts @@ -0,0 +1,28 @@ +/** + * Whether `path` matches `pattern`, with the rules of a shell glob as + * Python's fnmatch has them: `*` is any text (a `/` too), `?` is one + * character, `[abc]` is one of the characters. + */ +export function fnmatch(path: string, pattern: string): boolean { + let expression = ""; + for (let index = 0; index < pattern.length; index++) { + const char = pattern[index] ?? ""; + if (char === "*") { + expression += ".*"; + } else if (char === "?") { + expression += "."; + } else if (char === "[") { + const end = pattern.indexOf("]", index + 1); + if (end === -1) { + expression += "\\["; + } else { + const body = pattern.slice(index + 1, end).replaceAll("\\", "\\\\"); + expression += "[" + (body.startsWith("!") ? "^" + body.slice(1) : body) + "]"; + index = end; + } + } else { + expression += char.replace(/[.+^${}()|\\\]]/g, "\\$&"); + } + } + return new RegExp(`^${expression}$`, "s").test(path); +} diff --git a/packages/codeupipe-lint-ts/src/format-report-filter.ts b/packages/codeupipe-lint-ts/src/format-report-filter.ts new file mode 100644 index 0000000..43522f4 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/format-report-filter.ts @@ -0,0 +1,29 @@ +import type { Filter, Payload } from "codeupipe"; + +import { formatFinding } from "./finding.ts"; +import type { Finding } from "./finding.ts"; + +/** + * The findings as sorted report lines, and the exit code. + * + * Input keys: `findings`, `suppressed`, `errors`. + * Output keys: `findings` (sorted), `report` (text for stdout), + * `errorReport` (text for stderr), `exitCode`: 0 no findings, 1 findings, + * 2 a path could not be read, so the check is not complete. + */ +export class FormatReportFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])].sort((a, b) => + (a.path < b.path ? -1 : a.path > b.path ? 1 : 0) || a.line - b.line || a.col - b.col + || (a.rule < b.rule ? -1 : a.rule > b.rule ? 1 : 0)); + const suppressed = payload.get("suppressed") ?? []; + const errors = payload.get("errors") ?? []; + const lines = findings.map(formatFinding); + let summary = findings.length === 0 ? "No findings" : `${findings.length} ${findings.length === 1 ? "finding" : "findings"}`; + if (suppressed.length > 0) summary += ` (${suppressed.length} suppressed)`; + lines.push(summary + "."); + return payload.insert("findings", findings).insert("report", lines.join("\n")) + .insert("errorReport", errors.map((error) => "error: " + error).join("\n")) + .insert("exitCode", errors.length > 0 ? 2 : findings.length > 0 ? 1 : 0); + } +} diff --git a/packages/codeupipe-lint-ts/src/index.ts b/packages/codeupipe-lint-ts/src/index.ts new file mode 100644 index 0000000..c0e9c9d --- /dev/null +++ b/packages/codeupipe-lint-ts/src/index.ts @@ -0,0 +1,5 @@ +/** codeupipe-lint: a linter for TypeScript code that uses codeupipe. */ +export type { Finding } from "./finding.ts"; +export { lintPaths } from "./lint-paths.ts"; +export { LintPipeline } from "./lint-pipeline.ts"; +export { RULES } from "./rules.ts"; diff --git a/packages/codeupipe-lint-ts/src/lint-paths.ts b/packages/codeupipe-lint-ts/src/lint-paths.ts new file mode 100644 index 0000000..a6415bb --- /dev/null +++ b/packages/codeupipe-lint-ts/src/lint-paths.ts @@ -0,0 +1,13 @@ +import { Payload } from "codeupipe"; + +import { LintPipeline } from "./lint-pipeline.ts"; + +/** + * Run the linter over `paths`. The Payload that comes back holds `findings`, + * `suppressed`, `errors`, `report`, `errorReport` and `exitCode`. + */ +export function lintPaths(paths: string[], options: { exclude?: string[]; disable?: string[] } = {}): Promise { + return new LintPipeline().run(new Payload({ + paths, findings: [], errors: [], exclude: options.exclude ?? [], disable: options.disable ?? [], + })); +} diff --git a/packages/codeupipe-lint-ts/src/lint-pipeline.ts b/packages/codeupipe-lint-ts/src/lint-pipeline.ts new file mode 100644 index 0000000..f472e70 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/lint-pipeline.ts @@ -0,0 +1,29 @@ +import { Pipeline } from "codeupipe"; + +import { AnalyzeOutsideWritesFilter } from "./analyze-outside-writes-filter.ts"; +import { AnalyzePrimitiveNamesFilter } from "./analyze-primitive-names-filter.ts"; +import { AnalyzePrimitivesPerFileFilter } from "./analyze-primitives-per-file-filter.ts"; +import { AnalyzeStateReadsFilter } from "./analyze-state-reads-filter.ts"; +import { AnalyzeStepsFilter } from "./analyze-steps-filter.ts"; +import { ApplySuppressionsFilter } from "./apply-suppressions-filter.ts"; +import { CollectFilesFilter } from "./collect-files-filter.ts"; +import { DropDisabledFilter } from "./drop-disabled-filter.ts"; +import { FormatReportFilter } from "./format-report-filter.ts"; +import { ParseFilesFilter } from "./parse-files-filter.ts"; + +/** The linter, as a codeupipe pipeline. */ +export class LintPipeline extends Pipeline { + constructor() { + super(); + this.addFilter(new CollectFilesFilter()); // paths to files + this.addFilter(new ParseFilesFilter()); // files to syntax trees (CUP-I000) + this.addFilter(new AnalyzeStepsFilter()); // CUP-A001, A002, A003, A005, A008, A009 + this.addFilter(new AnalyzeStateReadsFilter()); // CUP-I003 + this.addFilter(new AnalyzeOutsideWritesFilter()); // CUP-I004 + this.addFilter(new AnalyzePrimitivesPerFileFilter()); // CUP-I005 + this.addFilter(new AnalyzePrimitiveNamesFilter()); // CUP-I006, CUP-I007 + this.addFilter(new DropDisabledFilter()); + this.addFilter(new ApplySuppressionsFilter()); + this.addFilter(new FormatReportFilter()); + } +} diff --git a/packages/codeupipe-lint-ts/src/parse-files-filter.ts b/packages/codeupipe-lint-ts/src/parse-files-filter.ts new file mode 100644 index 0000000..3f30eee --- /dev/null +++ b/packages/codeupipe-lint-ts/src/parse-files-filter.ts @@ -0,0 +1,45 @@ +import { readFileSync } from "node:fs"; + +import type { Filter, Payload } from "codeupipe"; +import ts from "typescript"; + +import type { Finding } from "./finding.ts"; +import { PARSE_ERROR, RULES } from "./rules.ts"; +import type { Source } from "./source.ts"; + +/** + * Reads and parses each file. + * + * Input keys: `files`, `findings`, `errors`. + * Output keys: `sources` (the parsed files), `findings` with CUP-I000 for a + * file that has a syntax error, `errors` for a file that cannot be read. + */ +export class ParseFilesFilter implements Filter { + call(payload: Payload): Payload { + const findings = [...(payload.get("findings") ?? [])]; + const errors = [...(payload.get("errors") ?? [])]; + const sources: Source[] = []; + for (const path of payload.get("files") ?? []) { + let text: string; + try { + text = readFileSync(path, "utf8"); + } catch (error) { + errors.push(`${path}: ${error instanceof Error ? error.message : String(error)}`); + continue; + } + const tree = ts.createSourceFile(path, text, ts.ScriptTarget.Latest, true); + const problem = ts.transpileModule(text, { fileName: path, reportDiagnostics: true }).diagnostics?.[0]; + if (problem !== undefined) { + const at = tree.getLineAndCharacterOfPosition(problem.start ?? 0); + const what = ts.flattenDiagnosticMessageText(problem.messageText, " "); + findings.push({ + path, line: at.line + 1, col: at.character + 1, rule: PARSE_ERROR, slug: RULES[PARSE_ERROR] ?? "", + message: `${what} Fix the syntax error so the file can be checked.`, + }); + continue; + } + sources.push({ path, text, tree }); + } + return payload.insert("sources", sources).insert("findings", findings).insert("errors", errors); + } +} diff --git a/packages/codeupipe-lint-ts/src/primitive-kind.ts b/packages/codeupipe-lint-ts/src/primitive-kind.ts new file mode 100644 index 0000000..6613e77 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/primitive-kind.ts @@ -0,0 +1,84 @@ +import ts from "typescript"; + +export type PrimitiveKind = "Pipeline" | "Hook" | "Filter" | "Tap" | "Predicate"; + +const BY_METHOD: Array<[string, PrimitiveKind]> = [["call", "Filter"], ["observe", "Tap"], ["test", "Predicate"]]; +const BY_INTERFACE: Record = { Filter: "Filter", Tap: "Tap", Predicate: "Predicate" }; +const HOOK_METHODS = ["before", "after", "onError"]; + +function heritage(node: ts.ClassLikeDeclaration, token: ts.SyntaxKind): string[] { + const names: string[] = []; + for (const clause of node.heritageClauses ?? []) { + if (clause.token !== token) continue; + for (const type of clause.types) { + const expression = type.expression; + if (ts.isIdentifier(expression)) names.push(expression.text); + else if (ts.isPropertyAccessExpression(expression)) names.push(expression.name.text); + } + } + return names; +} + +/** The methods of a class, by name. */ +export function methodsOf(node: ts.ClassLikeDeclaration): Map { + const methods = new Map(); + for (const member of node.members) { + if (ts.isMethodDeclaration(member) && ts.isIdentifier(member.name)) methods.set(member.name.text, member); + } + return methods; +} + +/** One parameter that the caller must give: not optional, no default, not a rest parameter. */ +function takesOnlyThePayload(method: ts.MethodDeclaration): boolean { + const [parameter, ...others] = method.parameters; + return parameter !== undefined && others.length === 0 && parameter.dotDotDotToken === undefined + && parameter.questionToken === undefined && parameter.initializer === undefined; +} + +/** + * Which primitive a class is, read from its shape. Undefined for any other class. + * + * - `extends Pipeline` is a Pipeline. + * - `extends Hook`, or the methods `before`, `after` and `onError`, is a Hook. + * - `implements Filter`, or `call` with one parameter, is a Filter. + * - `implements Tap`, or `observe` with one parameter, is a Tap. + * - `implements Predicate`, or `test` with one parameter, is a Predicate. + * + * `call`, `observe` and `test` count only with exactly one required + * parameter, so an unrelated `call(a, b)` or `observe(options = {})` is not + * mistaken for a step. + */ +export function primitiveKind(node: ts.ClassLikeDeclaration): PrimitiveKind | undefined { + const methods = methodsOf(node); + const bases = heritage(node, ts.SyntaxKind.ExtendsKeyword); + if (bases.includes("Pipeline")) return "Pipeline"; + if (bases.includes("Hook") || HOOK_METHODS.every((name) => methods.has(name))) return "Hook"; + for (const name of heritage(node, ts.SyntaxKind.ImplementsKeyword)) { + const kind = BY_INTERFACE[name]; + if (kind !== undefined) return kind; + } + for (const [name, kind] of BY_METHOD) { + const method = methods.get(name); + if (method !== undefined && takesOnlyThePayload(method)) return kind; + } + return undefined; +} + +/** A test file, or a file under a `tests` directory. */ +export function isTestFile(path: string): boolean { + const parts = path.replaceAll("\\", "/").split("/"); + const name = parts[parts.length - 1] ?? ""; + return parts.slice(0, -1).some((part) => part === "tests" || part === "__tests__") + || /\.(test|spec)\.[cm]?tsx?$/.test(name); +} + +/** The classes at the top level of a file that are primitives. */ +export function topLevelPrimitives(tree: ts.SourceFile): Array<{ node: ts.ClassDeclaration; name: string; kind: PrimitiveKind }> { + const found: Array<{ node: ts.ClassDeclaration; name: string; kind: PrimitiveKind }> = []; + for (const statement of tree.statements) { + if (!ts.isClassDeclaration(statement) || statement.name === undefined) continue; + const kind = primitiveKind(statement); + if (kind !== undefined) found.push({ node: statement, name: statement.name.text, kind }); + } + return found; +} diff --git a/packages/codeupipe-lint-ts/src/read-config.ts b/packages/codeupipe-lint-ts/src/read-config.ts new file mode 100644 index 0000000..17bddcc --- /dev/null +++ b/packages/codeupipe-lint-ts/src/read-config.ts @@ -0,0 +1,43 @@ +import { existsSync, readFileSync } from "node:fs"; + +export const CONFIG_NAME = "cup.config.json"; +const LINT_KEYS = ["paths", "exclude", "disable"]; + +export type LintConfig = { paths?: string[]; exclude?: string[]; disable?: string[] }; + +/** + * The `lint` section of the config file, checked. Undefined when the file + * is not there. With `path` given, a file that is not there is an error. + * + * A key that the linter does not read is an error, so the file cannot hold a + * setting that does nothing. Other sections of the file are for other tools. + */ +export function readLintConfig(path?: string): LintConfig | undefined { + const name = path ?? CONFIG_NAME; + if (!existsSync(name)) { + if (path === undefined) return undefined; + throw new Error(`${name}: no such file`); + } + let config: unknown; + try { + config = JSON.parse(readFileSync(name, "utf8")); + } catch (error) { + throw new Error(`${name} is not valid JSON: ${error instanceof Error ? error.message : String(error)}`); + } + if (typeof config !== "object" || config === null || Array.isArray(config)) { + throw new Error(`${name} must hold a JSON object`); + } + const lint = (config as Record).lint ?? {}; + if (typeof lint !== "object" || lint === null || Array.isArray(lint)) { + throw new Error(`${name}: lint must be an object`); + } + for (const [key, value] of Object.entries(lint)) { + if (!LINT_KEYS.includes(key)) { + throw new Error(`${name}: lint: unknown key '${key}' (the keys are ${LINT_KEYS.join(", ")})`); + } + if (!Array.isArray(value) || !value.every((item) => typeof item === "string")) { + throw new Error(`${name}: lint.${key} must be a list of strings`); + } + } + return lint as LintConfig; +} diff --git a/packages/codeupipe-lint-ts/src/rules.ts b/packages/codeupipe-lint-ts/src/rules.ts new file mode 100644 index 0000000..af7d9d5 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/rules.ts @@ -0,0 +1,68 @@ +import type { Finding } from "./finding.ts"; +import { position } from "./source.ts"; +import type { Source } from "./source.ts"; +import type ts from "typescript"; + +export const PARSE_ERROR = "CUP-I000"; +export const FUNCTION_STEP = "CUP-A001"; +export const NAMED_FUNCTION_STEP = "CUP-A002"; +export const LOCAL_TYPE_STEP = "CUP-A003"; +export const UNNAMED_PIPELINE = "CUP-A005"; +export const FUNCTION_PREDICATE = "CUP-A008"; +export const OBJECT_LITERAL_STEP = "CUP-A009"; +export const STATE_READ = "CUP-I003"; +export const WRITE_OUTSIDE_FILTER = "CUP-I004"; +export const ONE_PRIMITIVE_PER_FILE = "CUP-I005"; +export const PRIMITIVE_FILE_NAME = "CUP-I006"; +export const PRIMITIVE_ROLE_SUFFIX = "CUP-I007"; + +/** Rule id to its short name. The ids and names are those of the Python linter, where it has the rule. */ +export const RULES: Record = { + [PARSE_ERROR]: "parse-error", + [FUNCTION_STEP]: "lambda-step", + [NAMED_FUNCTION_STEP]: "local-function-step", + [LOCAL_TYPE_STEP]: "local-type-step", + [UNNAMED_PIPELINE]: "unnamed-pipeline-step", + [FUNCTION_PREDICATE]: "function-predicate", + [OBJECT_LITERAL_STEP]: "object-literal-step", + [STATE_READ]: "state-read-in-step", + [WRITE_OUTSIDE_FILTER]: "write-outside-filter", + [ONE_PRIMITIVE_PER_FILE]: "one-primitive-per-file", + [PRIMITIVE_FILE_NAME]: "primitive-file-name", + [PRIMITIVE_ROLE_SUFFIX]: "primitive-role-suffix", +}; + +const NAMED_CLASS = "Define it at module level as a named class (a Valve predicate: a class with test(payload)) " + + "so it has a name in State and can be tested on its own."; + +const FIX: Record = { + [PARSE_ERROR]: "Fix the syntax error so the file can be checked.", + [FUNCTION_STEP]: NAMED_CLASS, + [NAMED_FUNCTION_STEP]: NAMED_CLASS, + [LOCAL_TYPE_STEP]: NAMED_CLASS, + [OBJECT_LITERAL_STEP]: NAMED_CLASS, + [UNNAMED_PIPELINE]: "A nested pipeline must be a named pipeline type (class UserOnboarding extends Pipeline), " + + "named like a filter. A plain Pipeline can only run at the top level.", + [FUNCTION_PREDICATE]: "Valve predicates are objects (conformance/API.md, \"Valve\"): wrap the condition in a " + + "named class with test(payload), configuration on the instance, and pass an instance.", + [STATE_READ]: "A Filter's result and a Valve's decision must depend only on the Payload. Put what the step " + + "needs into the Payload, or observe State from a Tap or a Hook.", + [WRITE_OUTSIDE_FILTER]: "Only a Filter writes to the Payload, and nothing at run time stops this write. " + + "Put the write in a Filter, or write to payload.copy().", + [ONE_PRIMITIVE_PER_FILE]: "Give each Filter, Predicate, Tap, Hook and named Pipeline its own file, so it can " + + "be tested alone and used again. The pipeline file is where they come together.", + [PRIMITIVE_FILE_NAME]: "The file name of a primitive is its class name in kebab-case, so one name gives the other.", + [PRIMITIVE_ROLE_SUFFIX]: "A primitive's class name ends with its role (Filter, Tap, Hook, Predicate, " + + "Pipeline), so the name tells what the class is.", +}; + +/** A Finding at `node`. Its message reads `. `. */ +export function makeFinding(source: Source, node: ts.Node, rule: string, what: string, callNode?: ts.Node): Finding { + const { line, col } = position(source, node); + const finding: Finding = { path: source.path, line, col, rule, slug: RULES[rule] ?? "", message: `${what}. ${FIX[rule]}` }; + if (callNode !== undefined) { + const callLine = position(source, callNode).line; + if (callLine !== line) finding.callLine = callLine; + } + return finding; +} diff --git a/packages/codeupipe-lint-ts/src/source.ts b/packages/codeupipe-lint-ts/src/source.ts new file mode 100644 index 0000000..f542e71 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/source.ts @@ -0,0 +1,10 @@ +import ts from "typescript"; + +/** One parsed file. */ +export type Source = { path: string; text: string; tree: ts.SourceFile }; + +/** The line and column of a node, from 1. */ +export function position(source: Source, node: ts.Node): { line: number; col: number } { + const at = source.tree.getLineAndCharacterOfPosition(node.getStart(source.tree)); + return { line: at.line + 1, col: at.character + 1 }; +} diff --git a/packages/codeupipe-lint-ts/src/step-classifier.ts b/packages/codeupipe-lint-ts/src/step-classifier.ts new file mode 100644 index 0000000..bb95d23 --- /dev/null +++ b/packages/codeupipe-lint-ts/src/step-classifier.ts @@ -0,0 +1,125 @@ +import ts from "typescript"; + +import { + FUNCTION_PREDICATE, FUNCTION_STEP, LOCAL_TYPE_STEP, NAMED_FUNCTION_STEP, OBJECT_LITERAL_STEP, UNNAMED_PIPELINE, +} from "./rules.ts"; + +export type Role = "Filter" | "Tap" | "Pipeline" | "Valve predicate" | "Valve filter"; + +/** A rule, and the words that say what the value is: `is a function`. */ +export type Verdict = { rule: string; what: string }; + +function unwrap(node: ts.Expression): ts.Expression { + let inner = node; + while (ts.isParenthesizedExpression(inner) || ts.isAsExpression(inner) || ts.isNonNullExpression(inner) + || ts.isSatisfiesExpression(inner)) { + inner = inner.expression; + } + return inner; +} + +function isScope(node: ts.Node): boolean { + return ts.isSourceFile(node) || ts.isFunctionLike(node) || ts.isModuleBlock(node) || ts.isClassStaticBlockDeclaration(node); +} + +/** The declarations of `name` that the scope `scope` holds itself, not in a function inside it. */ +function declarationsIn(scope: ts.Node, name: string): ts.Node[] { + const found: ts.Node[] = []; + if (ts.isFunctionLike(scope)) { + for (const parameter of scope.parameters) { + if (parameter.getText().includes(name)) found.push(parameter); + } + } + const visit = (node: ts.Node) => { + if (ts.isVariableDeclaration(node) || ts.isBindingElement(node)) { + if (ts.isIdentifier(node.name) ? node.name.text === name : node.name.getText().includes(name)) found.push(node); + } else if ((ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node)) && node.name?.text === name) { + found.push(node); + } else if ((ts.isImportClause(node) || ts.isImportSpecifier(node) || ts.isNamespaceImport(node)) + && node.name?.text === name) { + found.push(node); + } else if (ts.isCatchClause(node) && node.variableDeclaration?.name.getText() === name) { + found.push(node); + } + if (node !== scope && isScope(node)) return; + ts.forEachChild(node, visit); + }; + ts.forEachChild(scope, visit); + return found; +} + +/** + * The one declaration that `name` has at `from`, by the rules of scope: the + * nearest function outwards to the file. Undefined when the file does not + * bind the name, or binds it in a way that this linter does not follow. + */ +function resolve(name: ts.Identifier): ts.Node | undefined { + for (let scope: ts.Node | undefined = name.parent; scope !== undefined; scope = scope.parent) { + if (!isScope(scope)) continue; + const found = declarationsIn(scope, name.text); + if (found.length === 1) return found[0]; + if (found.length > 1) return undefined; + } + return undefined; +} + +function isInsideFunction(node: ts.Node): boolean { + for (let parent = node.parent; parent !== undefined; parent = parent.parent) { + if (ts.isFunctionLike(parent)) return true; + } + return false; +} + +function isConst(declaration: ts.VariableDeclaration): boolean { + return ts.isVariableDeclarationList(declaration.parent) && (declaration.parent.flags & ts.NodeFlags.Const) !== 0; +} + +/** + * What is wrong with a value that is given as a step, or undefined. + * + * A name is followed to its declaration in the same file when that is a + * `const`, a function or a class. A `let`, a parameter, an import and the + * result of a call are not followed. + */ +export function classifyStep(expression: ts.Expression, role: Role, seen: Set = new Set()): Verdict | undefined { + const node = unwrap(expression); + if (seen.has(node)) return undefined; + seen.add(node); + + if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) { + return { rule: FUNCTION_STEP, what: "is a function" }; + } + if (ts.isObjectLiteralExpression(node)) { + return { rule: OBJECT_LITERAL_STEP, what: "is an object literal" }; + } + if (ts.isNewExpression(node)) { + const type = unwrap(node.expression); + if (ts.isClassExpression(type)) { + return { rule: LOCAL_TYPE_STEP, what: "is an instance of a class with no name" }; + } + if (ts.isIdentifier(type)) { + if (type.text === "Pipeline" && role === "Pipeline") { + return { rule: UNNAMED_PIPELINE, what: "is a plain `new Pipeline()`" }; + } + const declaration = resolve(type); + if (declaration !== undefined && ts.isClassDeclaration(declaration) && isInsideFunction(declaration)) { + return { rule: LOCAL_TYPE_STEP, what: `is an instance of \`${type.text}\`, a class that is declared in a function` }; + } + } + return undefined; + } + if (ts.isIdentifier(node)) { + const declaration = resolve(node); + if (declaration === undefined) return undefined; + if (ts.isFunctionDeclaration(declaration)) { + return role === "Valve predicate" + ? { rule: FUNCTION_PREDICATE, what: `\`${node.text}\` is a function, not a predicate object` } + : { rule: NAMED_FUNCTION_STEP, what: `\`${node.text}\` is a function` }; + } + if (ts.isVariableDeclaration(declaration) && isConst(declaration) && declaration.initializer !== undefined) { + const verdict = classifyStep(declaration.initializer, role, seen); + return verdict === undefined ? undefined : { rule: verdict.rule, what: `\`${node.text}\` ${verdict.what}` }; + } + } + return undefined; +} diff --git a/packages/codeupipe-lint-ts/tests/anonymous-steps.test.ts b/packages/codeupipe-lint-ts/tests/anonymous-steps.test.ts new file mode 100644 index 0000000..1de5589 --- /dev/null +++ b/packages/codeupipe-lint-ts/tests/anonymous-steps.test.ts @@ -0,0 +1,126 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { lintIn } from "./lint-in.ts"; + +const HEAD = `import { Pipeline } from "codeupipe"; +class AddFilter { call(payload) { return payload; } } +class IsOnPredicate { test(payload) { return true; } } +const pipeline = new Pipeline(); +`; + +test("named classes at module level are clean", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +pipeline.addFilter(new AddFilter()); +pipeline.addValve(new IsOnPredicate(), new AddFilter()); +const kept = new AddFilter(); +pipeline.addFilter(kept); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, []); + assert.equal(out.exitCode, 0); + assert.match(out.report, /^No findings\.$/); +}); + +test("CUP-A001: an arrow function or a function expression as a step", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +pipeline.addFilter((payload) => payload); +pipeline.addTap(function (payload) {}); +const later = (payload) => payload; +pipeline.addFilter(later); +pipeline.addValve((payload) => true, new AddFilter()); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:6 CUP-A001", "build.ts:7 CUP-A001", "build.ts:9 CUP-A001", "build.ts:10 CUP-A001"]); + assert.match(out.findings[0]?.message ?? "", /^Filter is a function\./); + assert.match(out.findings[2]?.message ?? "", /^Filter `later` is a function\./); + assert.match(out.findings[3]?.message ?? "", /^Valve predicate is a function\./); + assert.equal(out.exitCode, 1); +}); + +test("CUP-A002: a named function as a Filter or a Tap", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +function normalize(payload) { return payload; } +pipeline.addFilter(normalize); +pipeline.addTap(normalize); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:7 CUP-A002", "build.ts:8 CUP-A002"]); +}); + +test("CUP-A003: a class that is declared in a function, or that has no name", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +function make() { + class LocalFilter { call(payload) { return payload; } } + pipeline.addFilter(new LocalFilter()); + const kept = new LocalFilter(); + pipeline.addFilter(kept); +} +pipeline.addFilter(new (class { call(payload) { return payload; } })()); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:8 CUP-A003", "build.ts:10 CUP-A003", "build.ts:12 CUP-A003"]); +}); + +test("CUP-A005: a plain Pipeline as a step", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +class InnerPipeline extends Pipeline {} +pipeline.addPipeline(new Pipeline()); +const inner = new Pipeline(); +pipeline.addPipeline(inner); +pipeline.addPipeline(new InnerPipeline()); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:7 CUP-A005", "build.ts:9 CUP-A005"]); +}); + +test("CUP-A008: a named function as a Valve predicate", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +function isOn(payload) { return true; } +pipeline.addValve(isOn, new AddFilter()); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:7 CUP-A008"]); +}); + +test("CUP-A009: an object literal as a step", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +pipeline.addFilter({ call(payload) { return payload; } }); +const tap = { observe(payload) {} }; +pipeline.addTap(tap); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:6 CUP-A009", "build.ts:8 CUP-A009"]); +}); + +test("a name that the file does not bind, and a let, are not flagged", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +import { imported } from "./other.ts"; +let changed = (payload) => payload; +changed = new AddFilter(); +pipeline.addFilter(imported); +pipeline.addFilter(changed); +function use(parameter) { pipeline.addFilter(parameter); } +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, []); +}); + +test("this.addFilter in a pipeline class is checked too", async () => { + const out = await lintIn({ "pricing-pipeline.ts": `import { Pipeline } from "codeupipe"; +export class PricingPipeline extends Pipeline { + constructor() { + super(); + this.addFilter((payload) => payload); + } +} +` }); + assert.deepEqual(out.found, ["pricing-pipeline.ts:5 CUP-A001"]); +}); + +test("cup: allow-anonymous suppresses a finding on its line, the line above, or the line of the call", async () => { + const out = await lintIn({ "build.ts": HEAD + ` +pipeline.addFilter((payload) => payload); // cup: allow-anonymous a test of the refusal +// cup: allow-anonymous +pipeline.addFilter((payload) => payload); +pipeline.addValve( // cup: allow-anonymous + (payload) => true, + new AddFilter()); +pipeline.addFilter((payload) => payload); +` }, { disable: ["CUP-I005", "CUP-I006"] }); + assert.deepEqual(out.found, ["build.ts:12 CUP-A001"]); + assert.equal(out.suppressed, 3); + assert.match(out.report, /1 finding \(3 suppressed\)\.$/); +}); diff --git a/packages/codeupipe-lint-ts/tests/cli.test.ts b/packages/codeupipe-lint-ts/tests/cli.test.ts new file mode 100644 index 0000000..b610724 --- /dev/null +++ b/packages/codeupipe-lint-ts/tests/cli.test.ts @@ -0,0 +1,81 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; + +import { lintIn } from "./lint-in.ts"; + +const CLI = join(import.meta.dirname, "../src/cli.ts"); +const BAD = `export class Discount { call(payload) { return payload; } }\n`; + +function project(files: Record): string { + const root = mkdtempSync(join(tmpdir(), "cup-lint-cli-")); + for (const [name, text] of Object.entries(files)) writeFileSync(join(root, name), text); + return root; +} + +function run(cwd: string, ...args: string[]) { + const done = spawnSync(process.execPath, [CLI, ...args], { cwd, encoding: "utf8" }); + return { code: done.status, out: done.stdout, err: done.stderr }; +} + +test("exit 0 with no findings, exit 1 with findings, one line for each", () => { + const root = project({ "discount.ts": BAD, "set-filter.ts": `export class SetFilter { call(payload) { return payload; } }\n` }); + const clean = run(root, "set-filter.ts"); + assert.equal(clean.code, 0); + assert.equal(clean.out.trim(), "No findings."); + const bad = run(root, "."); + assert.equal(bad.code, 1); + assert.match(bad.out, /^discount\.ts:1:14 CUP-I006 primitive-file-name /m); + assert.match(bad.out, /^2 findings\.$/m); +}); + +test("exit 2 for a path that is not there, and for a rule that is not known", () => { + const root = project({}); + assert.equal(run(root, "nothing-here").code, 2); + const unknown = run(root, "--disable", "CUP-X999", "."); + assert.equal(unknown.code, 2); + assert.match(unknown.err, /no such rule: CUP-X999/); + assert.equal(run(root).code, 2); +}); + +test("--rules lists the rule ids", () => { + const listed = run(project({}), "--rules"); + assert.equal(listed.code, 0); + assert.match(listed.out, /^CUP-A001 lambda-step$/m); + assert.match(listed.out, /^CUP-I007 primitive-role-suffix$/m); +}); + +test("--exclude and --disable", () => { + const root = project({ "discount.ts": BAD }); + assert.equal(run(root, "--exclude", "*/discount.ts", "--exclude", "discount.ts", ".").code, 0); + assert.equal(run(root, "--disable", "CUP-I006", "--disable", "CUP-I007", ".").code, 0); + assert.equal(run(root, "--disable", "CUP-I006", ".").code, 1); +}); + +test("with no paths, they come from cup.config.json", () => { + const root = project({ + "discount.ts": BAD, + "cup.config.json": JSON.stringify({ lint: { paths: ["."], disable: ["CUP-I006", "CUP-I007"] } }), + }); + assert.equal(run(root).code, 0); + assert.equal(run(root, "discount.ts").code, 1); + writeFileSync(join(root, "cup.config.json"), JSON.stringify({ lint: { path: ["."] } })); + const wrong = run(root); + assert.equal(wrong.code, 2); + assert.match(wrong.err, /unknown key 'path'/); +}); + +test("node_modules and dist are not read", async () => { + const out = await lintIn({ "node_modules/x/discount.ts": BAD, "dist/discount.ts": BAD, "types.d.ts": BAD }); + assert.deepEqual(out.found, []); +}); + +test("the linter is clean on its own source and on the core package", () => { + const here = join(import.meta.dirname, ".."); + const done = run(here, "src", "../codeupipe-ts/src", "../codeupipe-ts/examples", "../../conformance/runners/typescript"); + assert.equal(done.out.trim(), "No findings."); + assert.equal(done.code, 0); +}); diff --git a/packages/codeupipe-lint-ts/tests/invariants.test.ts b/packages/codeupipe-lint-ts/tests/invariants.test.ts new file mode 100644 index 0000000..01a0c13 --- /dev/null +++ b/packages/codeupipe-lint-ts/tests/invariants.test.ts @@ -0,0 +1,127 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { lintIn } from "./lint-in.ts"; + +test("CUP-I003: a Filter or a Predicate that reads State", async () => { + const out = await lintIn({ + "count-filter.ts": `import type { Payload, Pipeline, State } from "codeupipe"; +export class CountFilter { + pipeline: Pipeline; + seen: State; + state = 3; + constructor(pipeline: Pipeline, seen: State) { + this.pipeline = pipeline; + this.seen = seen; + } + call(payload: Payload): Payload { + payload.insert("a", this.pipeline.state.executed.length); + payload.insert("b", this.seen.skipped); + payload.insert("c", this.state); + payload.insert("d", payload.get("state")); + return payload; + } +} +`, + "ran-predicate.ts": `export class RanPredicate { + holder; + constructor(holder) { this.holder = holder; } + test(payload) { return this.holder.state.executed.length > 0; } +} +`, + "watch-tap.ts": `export class WatchTap { + pipeline; + observe(payload) { console.log(this.pipeline.state.executed); } +} +`, + }); + assert.deepEqual(out.found, ["count-filter.ts:11 CUP-I003", "count-filter.ts:12 CUP-I003", "ran-predicate.ts:4 CUP-I003"]); +}); + +test("CUP-I004: a write to the Payload from a Tap, a Predicate or a Hook", async () => { + const out = await lintIn({ + "mark-tap.ts": `export class MarkTap { + rows = []; + observe(payload) { + payload.insert("seen", true).remove("x"); + payload.copy().insert("mine", 1); + this.rows.insert(0, payload.get("a")); + } +} +`, + "is-new-predicate.ts": `export class IsNewPredicate { + test(p) { p.withTrace("t"); return true; } +} +`, + "stamp-hook.ts": `import { Hook } from "codeupipe"; +export class StampHook extends Hook { + override before(step, payload) { payload.merge(payload); } + override onError(step, error, payload) { payload.insert("failed", true); } +} +`, + "set-filter.ts": `export class SetFilter { + call(payload) { return payload.insert("a", 1); } +} +`, + }); + assert.deepEqual(out.found, ["is-new-predicate.ts:2 CUP-I004", "mark-tap.ts:4 CUP-I004", + "stamp-hook.ts:3 CUP-I004", "stamp-hook.ts:4 CUP-I004"]); + assert.match(out.findings[1]?.message ?? "", /^`observe` writes to the Payload \(`payload\.remove`\)\./); +}); + +test("CUP-I005: one primitive in each file", async () => { + const out = await lintIn({ + "set-filter.ts": `export class SetFilter { call(payload) { return payload; } } +class Helper {} +export class SeeTap { observe(payload) {} } +function make() { class LocalFilter { call(payload) { return payload; } } } +`, + "tests/steps.ts": `class AFilter { call(payload) { return payload; } } +class BFilter { call(payload) { return payload; } } +`, + "steps.test.ts": `class AFilter { call(payload) { return payload; } } +class BFilter { call(payload) { return payload; } } +`, + }); + assert.deepEqual(out.found, ["set-filter.ts:3 CUP-I005", "set-filter.ts:3 CUP-I006"]); + assert.match(out.findings[0]?.message ?? "", /^Tap `SeeTap` shares this file with Filter `SetFilter`\./); +}); + +test("CUP-I006: the file name is the class name in kebab-case", async () => { + const out = await lintIn({ + "discount.ts": `export class DiscountFilter { call(payload) { return payload; } }\n`, + "parse-html-filter.ts": `export class ParseHTMLFilter { call(payload) { return payload; } }\n`, + "pricing-pipeline.tsx": `import { Pipeline } from "codeupipe";\nexport class PricingPipeline extends Pipeline {}\n`, + }); + assert.deepEqual(out.found, ["discount.ts:1 CUP-I006"]); + assert.match(out.findings[0]?.message ?? "", /`DiscountFilter` is in `discount\.ts`\. Put it in `discount-filter\.ts`\./); +}); + +test("CUP-I007: the class name ends with its role", async () => { + const out = await lintIn({ + "discount.ts": `export class Discount { call(payload) { return payload; } }\n`, + "audit.ts": `import type { Tap } from "codeupipe";\nexport class Audit implements Tap { observe() {} }\n`, + "at-least.ts": `export class AtLeast { test(payload) { return true; } }\n`, + "print-steps.ts": `import { Hook } from "codeupipe";\nexport class PrintSteps extends Hook {}\n`, + "pricing.ts": `import { Pipeline } from "codeupipe";\nexport class Pricing extends Pipeline {}\n`, + "point.ts": `export class Point { call(a, b) {} test() {} observe(options = {}) {} }\n`, + }); + assert.deepEqual(out.found, ["at-least.ts:1 CUP-I006", "at-least.ts:1 CUP-I007", "audit.ts:2 CUP-I006", "audit.ts:2 CUP-I007", + "discount.ts:1 CUP-I006", "discount.ts:1 CUP-I007", "pricing.ts:2 CUP-I006", "pricing.ts:2 CUP-I007", + "print-steps.ts:2 CUP-I006", "print-steps.ts:2 CUP-I007"]); + assert.match(out.findings[1]?.message ?? "", /^Predicate `AtLeast` does not say what it is\. Name it `AtLeastPredicate`\./); +}); + +test("the comment markers suppress the invariant findings", async () => { + const out = await lintIn({ + "discount.ts": `// cup: allow-name\nexport class Discount { call(payload) { return payload; } }\n`, + "mark-tap.ts": `export class MarkTap {\n observe(payload) {\n payload.insert("a", 1); // cup: allow-outside-write\n }\n}\n`, + }); + assert.deepEqual(out.found, []); + assert.equal(out.suppressed, 3); +}); + +test("a file with a syntax error is CUP-I000", async () => { + const out = await lintIn({ "broken.ts": `export class {{{\n` }); + assert.deepEqual(out.found.map((line) => line.split(" ")[1]), ["CUP-I000"]); +}); diff --git a/packages/codeupipe-lint-ts/tests/lint-in.ts b/packages/codeupipe-lint-ts/tests/lint-in.ts new file mode 100644 index 0000000..4101b47 --- /dev/null +++ b/packages/codeupipe-lint-ts/tests/lint-in.ts @@ -0,0 +1,28 @@ +// A helper for the tests: write source text to a new directory and lint it. +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, relative } from "node:path"; + +import { lintPaths } from "../src/index.ts"; +import type { Finding } from "../src/index.ts"; + +export type Linted = { found: string[]; findings: Finding[]; suppressed: number; report: string; exitCode: number }; + +/** `found` holds one string for each finding: `file:line rule`. */ +export async function lintIn(files: Record, + options: { exclude?: string[]; disable?: string[] } = {}): Promise { + const root = mkdtempSync(join(tmpdir(), "cup-lint-")); + for (const [name, text] of Object.entries(files)) { + mkdirSync(dirname(join(root, name)), { recursive: true }); + writeFileSync(join(root, name), text); + } + const out = await lintPaths([root], options); + const findings = out.get("findings") ?? []; + return { + findings, + found: findings.map((f) => `${relative(root, f.path)}:${f.line} ${f.rule}`), + suppressed: (out.get("suppressed") ?? []).length, + report: out.get("report") ?? "", + exitCode: out.get("exitCode") ?? -1, + }; +} diff --git a/packages/codeupipe-lint-ts/tsconfig.build.json b/packages/codeupipe-lint-ts/tsconfig.build.json new file mode 100644 index 0000000..7f9bdff --- /dev/null +++ b/packages/codeupipe-lint-ts/tsconfig.build.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": false, + "rootDir": "src", + "outDir": "dist", + "declaration": true + }, + "include": ["src"] +} diff --git a/packages/codeupipe-lint-ts/tsconfig.json b/packages/codeupipe-lint-ts/tsconfig.json new file mode 100644 index 0000000..d05c8e5 --- /dev/null +++ b/packages/codeupipe-lint-ts/tsconfig.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022"], + "types": ["node"], + "strict": true, + "noUncheckedIndexedAccess": true, + "noImplicitOverride": true, + "erasableSyntaxOnly": true, + "verbatimModuleSyntax": true, + "allowImportingTsExtensions": true, + "rewriteRelativeImportExtensions": true, + "skipLibCheck": true, + "noEmit": true + }, + "include": ["src", "tests"] +} diff --git a/packages/codeupipe-ts/README.md b/packages/codeupipe-ts/README.md index 5bde8b8..49078fd 100644 --- a/packages/codeupipe-ts/README.md +++ b/packages/codeupipe-ts/README.md @@ -121,7 +121,8 @@ override the methods that you need. method, so `addFilter` refuses a function by its type. - **Two runs of one pipeline at the same time** each write to their own State. `pipeline.state` is the State of the run that started last. -- **This package has no export command and no linter.** Python has both. +- **There is no export command.** Python has one. The linter is the + package [codeupipe-lint-ts](../codeupipe-lint-ts/). ## Labels and minified code