Skip to content
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

Merged
merged 13 commits into from
Jul 16, 2024
Merged
18 changes: 18 additions & 0 deletions .github/workflows/_reusable-eng-tools-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ on:
sparse-checkout-paths:
description: Paths for sparse checkout
type: string
lint:
konrad-jamrozik marked this conversation as resolved.
Show resolved Hide resolved
description: Run 'npm run lint' if true
required: false
type: boolean
prettier:
description: Run 'npm run prettier' if true
required: false
type: boolean

jobs:
test:
Expand Down Expand Up @@ -53,6 +61,16 @@ jobs:
shell: pwsh
working-directory: ./eng/tools/${{ inputs.package }}

- run: npm run lint
if: inputs.lint == true
shell: pwsh
working-directory: ./eng/tools/${{ inputs.package }}

- run: npm run prettier
if: inputs.prettier == true
shell: pwsh
working-directory: ./eng/tools/${{ inputs.package }}

- run: npm run test:ci
shell: pwsh
working-directory: ./eng/tools/${{ inputs.package }}
Expand Down
27 changes: 27 additions & 0 deletions .github/workflows/specs-model-test.yaml
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.
Show resolved Hide resolved
prettier: true
10 changes: 8 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,18 @@ warnings.txt
.apitest
.assets

# Blanket ignores
*.js
*.d.ts
*.js.map
*.d.ts.map
*.bak

# Eng Tools
eng/tools/typespec-validation/dist
!eng/tools/typespec-validation/cmd/*.js
eng/tools/**/dist
!eng/tools/**/cmd/*.js
!eng/tools/**/eslint.config.js

# No package-lock.json files should be commited except the top-level.
**/package-lock.json
!/package-lock.json
14 changes: 14 additions & 0 deletions eng/.vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
konrad-jamrozik marked this conversation as resolved.
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"
]
}
50 changes: 50 additions & 0 deletions eng/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# `Eng` directory
konrad-jamrozik marked this conversation as resolved.
Show resolved Hide resolved

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 dependencies,
konrad-jamrozik marked this conversation as resolved.
Show resolved Hide resolved
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 upserted dependencies.
- In case any dependencies have been removed, 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 0.7.0`.

## 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].

[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
[microsoft/typespec package.json]: https://github.com/microsoft/typespec/blob/main/package.json
[microsoft/typespec .prettierrc.json]: https://github.com/microsoft/typespec/blob/main/.prettierrc.json
[eslint override]: https://github.com/Azure/azure-rest-api-specs/pull/29820#pullrequestreview-2177045580
[npm/cli #7384]: https://github.com/npm/cli/issues/7384
[`package-lock.json` from `main`]: https://github.com/Azure/azure-rest-api-specs/blob/main/package-lock.json
1 change: 1 addition & 0 deletions eng/tools/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "azure-rest-api-specs-eng-tools",
"devDependencies": {
"@azure-tools/specs-model": "file:specs-model",
"@azure-tools/suppressions": "file:suppressions",
"@azure-tools/tsp-client-tests": "file:tsp-client-tests",
"@azure-tools/typespec-requirement": "file:typespec-requirement",
Expand Down
4 changes: 4 additions & 0 deletions eng/tools/specs-model/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
*.json
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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
18 changes: 18 additions & 0 deletions eng/tools/specs-model/.prettierrc.mjs
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;
5 changes: 5 additions & 0 deletions eng/tools/specs-model/cmd/get-specs-model.js
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();
98 changes: 98 additions & 0 deletions eng/tools/specs-model/eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// @ts-check
konrad-jamrozik marked this conversation as resolved.
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
36 changes: 36 additions & 0 deletions eng/tools/specs-model/package.json
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"
}
}
7 changes: 7 additions & 0 deletions eng/tools/specs-model/src/getSpecsModel.ts
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}`;
}
32 changes: 32 additions & 0 deletions eng/tools/specs-model/src/index.ts
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 };
7 changes: 7 additions & 0 deletions eng/tools/specs-model/test/getSpecsModel.test.ts
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");
});
21 changes: 21 additions & 0 deletions eng/tools/specs-model/tsconfig.json
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
2 changes: 1 addition & 1 deletion eng/tools/suppressions/cmd/get-suppressions.js
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();
Loading
Loading