-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Add stub specs-model tool
#29820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add stub specs-model tool
#29820
Changes from 11 commits
c12d92b
803d6d1
2544825
25a1fbe
c266760
f9b17fc
fb05c72
f8b467e
8db902e
7275dd5
c6ae55c
3c0f9ae
8c9a961
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: Specs Model - Test | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| - typespec-next | ||
| pull_request: | ||
| paths: | ||
| - package-lock.json | ||
| - package.json | ||
| - tsconfig.json | ||
| - .github/workflows/_reusable-eng-tools-test.yaml | ||
| - .github/workflows/specs-model-test.yaml | ||
| - eng/tools/package.json | ||
| - eng/tools/tsconfig.json | ||
| - eng/tools/specs-model/** | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| specsModel: | ||
| name: Specs Model | ||
| uses: ./.github/workflows/_reusable-eng-tools-test.yaml | ||
| with: | ||
| package: specs-model | ||
| lint: true | ||
konrad-jamrozik marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| prettier: true | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| { | ||
konrad-jamrozik marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| "editor.defaultFormatter": "esbenp.prettier-vscode", | ||
| "[javascript]": { | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode" | ||
| }, | ||
| "editor.formatOnPaste": false, | ||
| "editor.formatOnType": false, | ||
| "editor.formatOnSave": true, | ||
| "editor.formatOnSaveMode": "file", // required to format on save | ||
| "prettier.requireConfig": true, | ||
| "cSpell.words": [ | ||
| "tseslint" | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # `eng` directory | ||
|
|
||
| The `eng` directory contains source code for automated tooling running on this repository pull requests. | ||
|
|
||
| For context on this directory, see [Design guidelines for spec repos validation tooling] (Microsoft-internal). | ||
|
|
||
| ## Code conventions | ||
|
|
||
| Below are code convention we strive to follow in `eng` directory: | ||
|
|
||
| ### package.json | ||
|
|
||
| - We align `package.json` dependencies versions across all `package.json` files. | ||
| - We align `package.json` dependencies numbers with [microsoft/typespec package.json]. | ||
| In few cases we allow more frequent update cadence. | ||
| - We avoid doing package overrides. For example, we use `v8` of `eslint` instead of `v9` to avoid an override. | ||
| [See this comment for details][eslint override]. | ||
| - We order `package.json` keys as follows: `name private type main bin scripts engines dependencies devDependencies`. | ||
konrad-jamrozik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ### package-lock.json | ||
|
|
||
| - We maintain only top-level `package-lock.json` file. Running `npm install` from top-level dir removes the need | ||
| to ever have other files. | ||
| - We ensure the lock file remains clean by ensuring that a PR that adds or modifies any `package.json` dependencies, | ||
| makes changes equivalent to following protocol: | ||
| - `cd <local-specs-clone-root>` | ||
| - `git clean -xdf` to remove all untracked files. | ||
| - Copy-over [`package-lock.json` from `main`] to local clone. | ||
| - `npm install` to reflect the added or modified dependencies. | ||
| - In case any dependencies have been removed from any `package.json`, we do `rm package-lock.json` and `npm install`. | ||
| This way we ensure the lock file remains free of unused dependencies. | ||
| - We do `npm update` only in stand-alone PRs. | ||
| - To avoid inconsistent content of `package-lock.json` due to bugs like [npm/cli #7384], | ||
| we ensure to use the same node and npm version to update `package-lock.json`. | ||
| As of 7/15/2024 this is `node 20.15.1` and `npm 10.7.0`. | ||
konrad-jamrozik marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ## Linting and prettier | ||
|
|
||
| - We make eslint-based linting rules and prettier mandatory in CI for all projects. | ||
| - We use strict rulesets as baseline. | ||
| - We discuss any desired rule divergences from the strict ruleset | ||
| and apply rule modifications to the configs with explanation for our decision. | ||
| - We align `prettier` rules with [microsoft/typespec .prettierrc.json]. | ||
|
|
||
| [`package-lock.json` from `main`]: https://github.com/Azure/azure-rest-api-specs/blob/main/package-lock.json | ||
| [Design guidelines for spec repos validation tooling]: https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/1153/Design-guidelines-for-spec-repos-validation-tooling | ||
| [eslint override]: https://github.com/Azure/azure-rest-api-specs/pull/29820#pullrequestreview-2177045580 | ||
| [microsoft/typespec .prettierrc.json]: https://github.com/microsoft/typespec/blob/main/.prettierrc.json | ||
| [microsoft/typespec package.json]: https://github.com/microsoft/typespec/blob/main/package.json | ||
| [npm/cli #7384]: https://github.com/npm/cli/issues/7384 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| *.json | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these config files be moved up into the tools directory so that we have a shared set for all our ts tools?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually, yes. But we will need to migrate each tool individually. |
||
| *.md | ||
| *.jsonc | ||
| dist | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // This config is adapted from https://github.com/microsoft/typespec/blob/main/.prettierrc.json | ||
| // See eng/README.md for context. | ||
| /** | ||
| * @see https://prettier.io/docs/en/configuration.html | ||
| * @type {import("prettier").Config} | ||
| */ | ||
| const config = { | ||
| arrowParens: "always", | ||
| trailingComma: "es5", | ||
| bracketSpacing: true, | ||
| endOfLine: "lf", | ||
| printWidth: 100, | ||
| semi: true, | ||
| singleQuote: false, | ||
| tabWidth: 2, | ||
| }; | ||
|
|
||
| export default config; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import { main } from "../dist/index.js"; | ||
|
|
||
| await main(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| // @ts-check | ||
konrad-jamrozik marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // The overall contents of this file is based on: | ||
| // https://typescript-eslint.io/getting-started#step-2-configuration | ||
| // https://typescript-eslint.io/getting-started/typed-linting/#shared-configurations | ||
| // Read inline comments for details on other sources. | ||
|
|
||
| import eslint from "@eslint/js"; | ||
| import eslintPluginUnicorn from "eslint-plugin-unicorn"; | ||
| import tseslint from "typescript-eslint"; | ||
|
|
||
| const config = tseslint.config( | ||
| // ======================================== | ||
| // ESLint + TS-ESLint configs | ||
| // ======================================== | ||
| { | ||
| // Needed for 'npm run lint' per: | ||
| // https://eslint.org/docs/latest/use/configure/migration-guide#--ext | ||
| // See also: | ||
| // - https://typescript-eslint.io/troubleshooting/typed-linting/#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file | ||
| // - https://eslint.org/docs/latest/use/configure/ignore | ||
| ignores: [".prettierrc.cjs", "**/*.d.ts", "**/*.js", "**/*.mjs"], | ||
| }, | ||
| eslint.configs.recommended, | ||
| ...tseslint.configs.strictTypeChecked, | ||
| ...tseslint.configs.stylisticTypeChecked, | ||
| { | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: true, | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| // Disable type-aware linting on .js files | ||
| // Otherwise eslint would complain about types in .js files, including this config file. | ||
| // Config snippet taken from https://typescript-eslint.io/packages/typescript-eslint/#advanced-usage | ||
| // Note: this is likely redundant with the global ignores of .js files, but keeping here for reference. | ||
| files: ["**/*.js"], | ||
| ...tseslint.configs.disableTypeChecked, | ||
| }, | ||
|
|
||
| // ======================================== | ||
| // Secondary configs | ||
| // ======================================== | ||
| // @ts-expect-error The unicorn configs are not typed correctly, but they do work. | ||
| // Snippet taken from https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs | ||
| eslintPluginUnicorn.configs["flat/recommended"], | ||
| // Note: in spite of my best efforts, I did not manage to get lodash eslint plugin to work in a clean way. | ||
| // https://github.com/wix-incubator/eslint-plugin-lodash | ||
| // I did manage to get it to lint, but the ESLint server output was throwing error about | ||
| // not being able to locate the config file. | ||
| // I suspect this is because the plugin was not migrated to the new flat config format since ESLint v9. | ||
| // Maybe this can be worked around with | ||
| // https://eslint.org/blog/2024/05/eslint-compatibility-utilities/ | ||
|
|
||
| // ======================================== | ||
| // Rulesets overrides | ||
| // ======================================== | ||
| { | ||
| rules: { | ||
| // Sometimes we have to help the type checker with "!": | ||
| // e.g. when doing `if (arr.length > 0) { const ... = arr[0]! }` | ||
| // Note: this originates from [strict] | ||
| // https://typescript-eslint.io/rules/no-non-null-assertion | ||
| "@typescript-eslint/no-non-null-assertion": "off", | ||
|
|
||
| // We want more flexibility with file names. | ||
| // https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/filename-case.md | ||
| "unicorn/filename-case": "off", | ||
|
|
||
| // We prefer to have explicitly import at the top of the file, even if the same element is exported again, | ||
| // which we do in index.ts files. | ||
| // https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-export-from.md | ||
| "unicorn/prefer-export-from": ["error", { ignoreUsedVariables: true }], | ||
|
|
||
| // We allow some abbreviations that we like. | ||
| // https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prevent-abbreviations.md | ||
| "unicorn/prevent-abbreviations": [ | ||
| "error", | ||
| { | ||
| allowList: { | ||
| args: true, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| export default config; | ||
|
|
||
| // Debug tool: | ||
| // Uncomment to print the config. View it in VS Code / Output / ESLint. Run "ESLint: Restart ESLint Server" command to force output. | ||
| // console.log(`ESLint config: ${JSON.stringify(config)}`) | ||
|
|
||
| // [strict]: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/strict.ts | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| { | ||
| "name": "@azure-tools/specs-model", | ||
| "private": true, | ||
| "type": "module", | ||
| "main": "dist/src/index.js", | ||
| "bin": { | ||
| "get-specs-model": "cmd/get-specs-model.js" | ||
| }, | ||
| "scripts": { | ||
| "build": "tsc", | ||
| "postinstall": "npm run build", | ||
| "test": "vitest", | ||
| "test:ci": "vitest run --coverage --reporter=verbose", | ||
| "lint": "eslint . -c eslint.config.js --report-unused-disable-directives --max-warnings 0", | ||
| "lint:fix": "eslint . -c eslint.config.js --fix", | ||
| "prettier": "prettier . --check", | ||
| "prettier:debug": "prettier . --check ---log-level debug", | ||
| "prettier:write": "prettier . --write" | ||
| }, | ||
| "engines": { | ||
| "node": ">= 18.0.0" | ||
| }, | ||
| "dependencies": {}, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.7.0", | ||
| "@tsconfig/strictest": "^2.0.5", | ||
| "@types/eslint__js": "^8.42.3", | ||
| "@types/node": "^18.19.31", | ||
| "@vitest/coverage-v8": "^1.6.0", | ||
| "eslint": "^8.57.0", | ||
| "eslint-plugin-unicorn": "^54.0.0", | ||
| "typescript": "~5.4.5", | ||
| "typescript-eslint": "^7.16.0", | ||
| "vitest": "^1.6.0" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export async function getSpecsModel(path: string): Promise<string> { | ||
| // eslint-disable-next-line @typescript-eslint/require-await | ||
| await (async () => { | ||
| console.log(path); | ||
| })(); | ||
| return `stub getSpecsModel. path: ${path}`; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { exit } from "node:process"; | ||
| import { getSpecsModel } from "./getSpecsModel.js"; | ||
|
|
||
| function getUsage(): string { | ||
| return ( | ||
| " Usage: npx get-specs-model <path-to-specs-file-or-directory>\n" + | ||
| "Returns: JSON metadata for the input file or directory.\n" + | ||
| "\n" + | ||
| "The input path:\n" + | ||
| "- Must be an absolute or relative path to local clone of the https://github.com/Azure/azure-rest-api-specs or https://github.com/Azure/azure-rest-api-specs-pr repo.\n" + | ||
| "- Must point to the <local_clone>/specification directory or one of its descendants.\n" + | ||
| "\n" + | ||
| "Example: npx get-specs-model $HOME/repos/azure-rest-api-specs/specification\n" + | ||
| "Returns: JSON with metadata for the entire 'specification' directory of the local clone of 'azure-rest-api-specs' repo.\n" | ||
| ); | ||
| } | ||
|
|
||
| export async function main() { | ||
| const args: string[] = process.argv.slice(2); | ||
|
|
||
| if (args.length > 0) { | ||
| const path: string = args[0]!; | ||
| const specsModel: string = await getSpecsModel(path); | ||
| console.log(JSON.stringify(specsModel)); | ||
| exit(0); | ||
| } else { | ||
| console.error(getUsage()); | ||
| exit(1); | ||
| } | ||
| } | ||
|
|
||
| export { getSpecsModel }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import { expect, test } from "vitest"; | ||
| import { getSpecsModel } from "../src/getSpecsModel.js"; | ||
|
|
||
| test("example getSpecsModel test", async () => { | ||
| const output = await getSpecsModel("foo_path"); | ||
| expect(output).toEqual("stub getSpecsModel. path: foo_path"); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| { | ||
| "extends": [ | ||
| "../tsconfig.json", | ||
| // [strictest] | ||
| "@tsconfig/strictest/tsconfig.json" | ||
| ], | ||
| "compilerOptions": { | ||
| "outDir": "./dist", | ||
| "target": "ES2022", | ||
| "lib": ["ES2022"], | ||
|
|
||
| // checkJS is set to true by [strictest] but we need to disable it to avoid this [build failure]. | ||
| // We don't need it anyway, as all sources are in .ts except the 3-line cmd entry-point. | ||
| // https://www.typescriptlang.org/tsconfig/#checkJs | ||
| "checkJs": false | ||
| } | ||
| } | ||
|
|
||
| // [strictest]: https://www.npmjs.com/package/@tsconfig/strictest from [tsconfig bases] | ||
| // [tsconfig bases]: https://github.com/tsconfig/bases#centralized-recommendations-for-tsconfig-bases from https://www.typescriptlang.org/tsconfig#target | ||
| // [build failure]: https://stackoverflow.com/questions/42609768/typescript-error-cannot-write-file-because-it-would-overwrite-input-file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import { main } from "../dist/src/index.js" | ||
| import { main } from "../dist/src/index.js"; | ||
|
|
||
| await main(); |
Uh oh!
There was an error while loading. Please reload this page.