feat: add Siemens REST API linter - #156
Draft
fh1ch wants to merge 33 commits into
Draft
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a new api-linter workspace package that provides a Spectral-based linter (rulesets + CLI + HTML report) aligned with the Siemens REST API Guidelines, along with Jest tests and OpenAPI fixture specs to validate the rules.
Changes:
- Added
api-linteras a workspace package in the rootpackage.json. - Implemented Spectral rulesets + custom JS functions for Siemens guideline checks (media type, versioning, security, pagination, etc.).
- Added Jest test suite plus OpenAPI fixture/spec files to verify rule behavior and CLI behavior.
Reviewed changes
Copilot reviewed 106 out of 106 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Registers api-linter as a workspace package. |
| api-linter/.npmignore | Excludes test/example directories from npm publish. |
| api-linter/README.md | Documents installation, CLI usage, and ruleset selection. |
| api-linter/package.json | Defines the @siemens/api-linter package metadata, dependencies, and scripts. |
| api-linter/src/argument.js | Implements CLI argument parsing and option defaults (including --resolve). |
| api-linter/src/extension.js | Exposes a JS API wrapper around Spectral ruleset loading and execution. |
| api-linter/src/templates/template.html | HTML template for generating lint reports. |
| api-linter/rulesets/semantic-versioning.yml | Adds semantic-versioning rule definition. |
| api-linter/rulesets/siemens-api.yml | All-in-one ruleset composition (includes spectral:oas). |
| api-linter/rulesets/siemens-api-express.yml | Composed ruleset without spectral:oas (guidelines-focused). |
| api-linter/rulesets/vs-extension.yml | Ruleset composition intended for VS/extension usage. |
| api-linter/rulesets/siemens-api-versioning.yml | Versioning ruleset entrypoint (extends semantic + custom logic). |
| api-linter/rulesets/siemens-api-versioning.js | Custom JS-driven rules for versioning behavior. |
| api-linter/rulesets/siemens-api-security.yml | Security ruleset entrypoint (extends custom JS logic). |
| api-linter/rulesets/siemens-api-security.js | Custom JS-driven security rule behavior. |
| api-linter/rulesets/siemens-api-media-type.yml | Media type and schema-structure rules. |
| api-linter/rulesets/siemens-api-error-reporting.yml | Error-reporting/status-code structure rules. |
| api-linter/rulesets/siemens-api-filtering.yml | Filtering/query-parameter rules. |
| api-linter/rulesets/siemens-api-sparse-fieldsets.yml | Sparse fieldsets rule(s). |
| api-linter/rulesets/siemens-api-pagination.yml | Pagination rules (links + strategies + meta). |
| api-linter/rulesets/siemens-api-sorting.yml | Sorting rule(s). |
| api-linter/rulesets/siemens-api-common-operation.yml | Common-operation rules (HTTP codes, headers, link structure). |
| api-linter/rulesets/functions/assert-http-codes-for-operation.js | Function helper for HTTP status-code assertions. |
| api-linter/rulesets/functions/count-resource-types.js | Function helper for counting resource types. |
| api-linter/rulesets/functions/get-resources-has-pagination-strategies-meta.js | Function helper for pagination meta strategy validation. |
| api-linter/rulesets/functions/get-resources-has-pagination-strategies.js | Function helper for pagination strategy validation. |
| api-linter/rulesets/functions/get-resources-has-pagination.js | Function helper for pagination links/key validation. |
| api-linter/rulesets/functions/get-resources-has-parameter.js | Function helper for checking presence of query parameters. |
| api-linter/rulesets/functions/get-status-code.js | Function helper for validating “individual vs collection” response codes. |
| api-linter/rulesets/functions/has-header-parameter-with-property-value.js | Function helper for checking header parameters exist. |
| api-linter/rulesets/functions/has-headers-with-property.js | Function helper for checking response headers exist. |
| api-linter/rulesets/functions/is-meta-object.js | Function helper for validating meta object type. |
| api-linter/rulesets/functions/is-object-schema.js | Function helper for validating object schemas. |
| api-linter/rulesets/functions/is-problem-json-schema.js | Function helper for validating problem+json schema shape. |
| api-linter/rulesets/functions/links-has-self.js | Function helper enforcing links.self presence. |
| api-linter/rulesets/functions/path-resource-name-is-lowercase-with-hyphen.js | Function helper enforcing path resource naming convention. |
| api-linter/rulesets/functions/query-parameters-length.js | Function helper for query parameter count/name rules. |
| api-linter/rulesets/functions/request-is-defined-document-structure-schema.js | Function helper enforcing request document structure. |
| api-linter/rulesets/functions/response-is-defined-document-structure-schema.js | Function helper enforcing response document structure. |
| api-linter/rulesets/functions/schema-test.js | Debug/test helper function (console logging). |
| api-linter/rulesets/functions/successful-response-status-code.js | Function helper for detecting 2xx success response presence. |
| api-linter/rulesets/functions/validate-b3-tracing.js | Function helper for B3 tracing header validation. |
| api-linter/test/base.js | Shared test helpers for severity filtering and assertions. |
| api-linter/test/api-commonops.test.js | Jest tests covering common-operation rules. |
| api-linter/test/api-errors-refs.test.js | Jest test covering external $ref resolution with resolver. |
| api-linter/test/api-errors.test.js | Jest tests for error-reporting rules. |
| api-linter/test/api-filtering.test.js | Jest tests for filtering rules. |
| api-linter/test/api-linter-cli.test.js | Jest tests for CLI behavior and external $ref resolution flag. |
| api-linter/test/api-mediatype.test.js | Jest tests for media-type rules. |
| api-linter/test/api-pagination.test.js | Jest tests for pagination rules. |
| api-linter/test/api-security.test.js | Jest tests for security rules. |
| api-linter/test/api-sparse-fieldsets.test.js | Jest tests for sparse fieldsets rule. |
| api-linter/test/api-versioning.test.js | Jest tests for versioning rules. |
| api-linter/test/testdata/commonops/.spectral.yml | Spectral config fixture for common-ops ruleset. |
| api-linter/test/testdata/commonops/800-1.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/800-2.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/800-4.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/801.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/801-4.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/802-6.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/803-2.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/803-6.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/commonops/804-1.yml | OpenAPI fixture for common-ops rule tests. |
| api-linter/test/testdata/error/.spectral.yml | Spectral config fixture for error ruleset. |
| api-linter/test/testdata/error/300.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/error/301.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/error/302.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/error/305.1.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/error/305.2.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/error/305.3.yml | OpenAPI fixture for error rules. |
| api-linter/test/testdata/fields/.spectral.yml | Spectral config fixture for sparse-fieldsets ruleset. |
| api-linter/test/testdata/fields/500.yml | OpenAPI fixture for sparse-fieldsets rule. |
| api-linter/test/testdata/filtering/.spectral.yml | Spectral config fixture for filtering ruleset. |
| api-linter/test/testdata/filtering/400.yml | OpenAPI fixture for filtering rule tests. |
| api-linter/test/testdata/filtering/401.yml | OpenAPI fixture for filtering rule tests. |
| api-linter/test/testdata/mediatype/.spectral.yml | Spectral config fixture for media-type ruleset. |
| api-linter/test/testdata/mediatype/100.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-1.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-2.1.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-2.2.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-4-1.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-7-2.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-8.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/mediatype/101-9.yml | OpenAPI fixture for media-type rule tests. |
| api-linter/test/testdata/pagination/.spectral.yml | Spectral config fixture for pagination ruleset. |
| api-linter/test/testdata/pagination/600.1.yml | OpenAPI fixture for pagination rule tests. |
| api-linter/test/testdata/pagination/600.2.yml | OpenAPI fixture for pagination rule tests. |
| api-linter/test/testdata/pagination/600.3.yml | OpenAPI fixture for pagination rule tests. |
| api-linter/test/testdata/pagination/601.yml | OpenAPI fixture for pagination rule tests. |
| api-linter/test/testdata/pagination/601-1-1.yml | OpenAPI fixture for pagination meta tests. |
| api-linter/test/testdata/pagination/601-2-1.yml | OpenAPI fixture for pagination meta tests. |
| api-linter/test/testdata/pagination/601-3-1.1.yml | OpenAPI fixture for pagination meta tests. |
| api-linter/test/testdata/pagination/601-3-1.2.yml | OpenAPI fixture for pagination meta tests. |
| api-linter/test/testdata/refs/.spectral.yml | Spectral config fixture for external-ref tests. |
| api-linter/test/testdata/refs/300.yml | OpenAPI fixture referencing external schemas. |
| api-linter/test/testdata/refs/common_errors.yml | External schema fixture used via $ref. |
| api-linter/test/testdata/security/.spectral.yml | Spectral config fixture for security ruleset. |
| api-linter/test/testdata/security/invalid.yml | OpenAPI fixture for security rule tests (missing auth header). |
| api-linter/test/testdata/security/valid.yml | OpenAPI fixture for security rule tests (has auth header). |
| api-linter/test/testdata/versioning/.spectral.yml | Spectral config fixture for versioning ruleset. |
| api-linter/test/testdata/versioning/200-1.yml | OpenAPI fixture for URL-based versioning checks. |
| api-linter/test/testdata/versioning/200-2.yml | OpenAPI fixture for header-based versioning checks. |
| api-linter/test/testdata/versioning/201.yml | OpenAPI fixture for response version header checks. |
| api-linter/test/testdata/versioning/semantic-versioning.yml | OpenAPI fixture for semantic-version string checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| expect(fs.existsSync(reportWith)).toBe(true); // report created | ||
| }); | ||
|
|
||
| test('without --resolve produces invalid-ref error', () => { |
Member
There was a problem hiding this comment.
I discussed this with Heron. He provided a fix but cannot push to this branch right now.
Bring the api-linter workspace under the repository-wide eslint configuration instead of its own bundled "standard" linter: - remove the api-linter eslint ignore and add a flat-config block that enables the node and jest globals for its CommonJS/ESM sources - add "globals" as a root devDependency (imported by eslint.config.js) - drop the now-unused "standard" dependency - drop unused devDependencies "jest-junit" and the duplicated "jsonpath"
- use block-scoped const for the repeated linkProps declarations to avoid redeclaration in get-resources-has-pagination - declare tempFilePath without a redundant initial assignment - use an optional catch binding for the unused cleanup error - iterate map values instead of destructuring an unused key
Add the SPDX copyright header required by the repository eslint header rule to all api-linter JavaScript sources.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 107 out of 108 changed files in this pull request and generated 1 comment.
Suppressed comments (17)
api-linter/test/api-linter-cli.test.js:66
- The test title says it "produces invalid-ref error", but the assertion expects
stdoutto not matchinvalid-ref. This makes the intent unclear and can mislead future maintainers; rename the test or update the expectation to match the intended behavior.
api-linter/src/extension.js:21 - The
Documentis created with a hard-coded source'openapi.yml'instead ofspecFilePath. This breaks relative path resolution for$refs and produces misleading diagnostics. Pass the real spec file path as the document source (and consider adding a resolver like the CLI does if external refs must work here too).
api-linter/rulesets/siemens-api-versioning.js:57 pattern'smatch/notMatchoptions are provided as strings including regex delimiters and flags (e.g.'/.../i'). In this repo, otherpatternusages pass the pattern without delimiters, and including the slashes can make the regex match the literal/characters instead of applying flags. Use a realRegExp(or a delimiter-free pattern string) so rule 200.1 matches correctly.
api-linter/rulesets/functions/get-status-code.js:26- This condition uses bitwise
&instead of logical&&, which can lead to incorrect truthiness and removes short-circuiting. Use&&here.
} else {
if ((options == 'individual') & (props.type == 'object')) {
return true;
} else if (options == 'collection' && props.type == 'array') {
return true;
}
api-linter/rulesets/functions/has-header-parameter-with-property-value.js:23
- The function returns success if any of the requested header parameters exists, but the caller (rule 803.6) passes multiple required headers (If-Match and If-None-Match). With the current logic, having only one header would incorrectly satisfy the rule. Track each expected header and report which ones are missing.
var params = targetVal.parameters;
var exist = false;
for (const param of params) {
for (const ops of options) {
if (param[ops['property']] === ops['value'] && param['in'] === 'header') {
exist = true;
}
}
}
if (!exist) {
results.push({ message: 'No header parameter provided in the options' });
}
api-linter/test/api-versioning.test.js:27
spectral.run(document).then(...)is not awaited/returned from the async Jest test, so the test can finish before the assertions run (false positives / unhandled rejections). Useawait spectral.run(...)(and apply the same change to the other tests in this file).
api-linter/test/api-pagination.test.js:26- The async test doesn't await/return the
spectral.run(...)promise, so Jest may mark the test complete before assertions run. Convert toconst results = await spectral.run(document)(and update the other tests in this file similarly).
api-linter/test/api-mediatype.test.js:28 spectral.run(document).then(...)isn't awaited/returned, so these Jest tests can pass without executing assertions. Useawait spectral.run(document)(and apply to all occurrences in this file).
api-linter/test/api-filtering.test.js:28- The test does not await/return the promise from
spectral.run(...), so assertions may run after the test completes. Useawait spectral.run(document)(and update the other test in this file as well).
api-linter/test/api-errors.test.js:29 spectral.run(...).then(...)isn't awaited/returned from the async test, so the test can complete before expectations execute. Preferconst results = await spectral.run(document);(and apply to all tests in this file).
api-linter/test/api-commonops.test.js:27- These async Jest tests never await/return the
spectral.run(...)promise, so assertions may not be part of the test lifecycle. Useawait spectral.run(document)(and apply across the file).
api-linter/test/api-security.test.js:19 spectral.run(...).then(...)isn't awaited/returned from these async Jest tests, so Jest may finish the test before expectations run. Useawait spectral.run(document)(and update the other tests in this file similarly).
api-linter/test/api-security.test.js:39No security problem test 2andNo security problem test 3are identical (same inputs and assertions), which adds runtime without increasing coverage. Remove one of them or change it to cover a distinct case.
api-linter/src/index.js:205--resolveis documented as "follow external $refs", but the linter usesspectral.run(..., { resolve: true }). Elsewhere in this repo external resolving is invoked via{ resolve: { external: true } }, so--resolvelikely doesn't do what it says. Use the explicit external resolve option so CLI behavior matches the flag name and tests.
api-linter/package.json:43@jamietanna/spectral-test-harnessis only used by tests underapi-linter/test, but it's listed underdependencies. This bloats the published package and installs test-only code for consumers. Move it todevDependencies.
"dependencies": {
"@stoplight/spectral-core": "^1.15.1",
"@stoplight/spectral-ruleset-bundler": "^1.5.2",
"@stoplight/spectral-ref-resolver": "^1.0.4",
"@jamietanna/spectral-test-harness": "^0.3.0",
"handlebars": "^4.7.7",
"commander": "^12.0.0",
"underscore": "^1.13.1",
"js-yaml": "^4.1.0",
"jsonpath": "^1.1.1"
},
"devDependencies": {
"jest": "^29.0.0"
}
api-linter/rulesets/functions/schema-test.js:29
- This Spectral function logs schemas via
console.log, which is noisy for consumers/CI if the function is ever enabled. Remove debug logging (and consider moving this helper to a test-only location if it's only for debugging).
const assertObjectSchema = schema => {
console.log('==========================================');
console.log(schema);
};
api-linter/test/api-sparse-fieldsets.test.js:20
spectral.run(document).then(...)isn't awaited/returned from the async Jest test, so assertions may run after the test completes. Useawait spectral.run(document)to ensure the test actually fails when expectations fail.
Member
Author
|
@toedter as discussed, over to you to address the remaining topics 🏓 Thanks a ton 🙇 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ens REST API guidelines"
… in template.html
…ith-property-value.js
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 111 out of 114 changed files in this pull request and generated no new comments.
Suppressed comments (11)
api-linter/test/api-linter-cli.test.js:56
- The test name says it "produces invalid-ref error" but the assertions expect
invalid-refNOT to appear, which is confusing when reading failures. Rename the test to match the expected behavior (or update expectations).
api-linter/test/api-versioning.test.js:21 - Jest won't wait for this Promise chain because it's neither awaited nor returned, so assertions inside the
.then()may never run (tests can pass incorrectly). Return/awaitspectral.run(...)here (and in the other tests in this file).
api-linter/test/api-mediatype.test.js:21 - This
spectral.run(...).then(...)Promise is not awaited/returned, so Jest may finish the test before assertions run. Return/await the Promise here (and for the otherspectral.runcalls in this file).
api-linter/test/api-pagination.test.js:20 - The Promise returned by
spectral.run(...)isn't awaited/returned, so assertions in the.then()can be skipped. Return/await it here (and for the other tests in this file).
api-linter/test/api-commonops.test.js:20 - This test doesn't await/return the Promise from
spectral.run(...), so Jest may report success without executing the assertions. Return/await the Promise here (and for the other tests in this file).
api-linter/test/api-filtering.test.js:20 - The
spectral.run(...).then(...)chain isn't awaited/returned, so Jest can finish early and skip these assertions. Return/await the Promise here (and for the other tests in this file).
api-linter/src/index.js:204 Spectral#runis invoked with{ resolve: true }, but elsewhere in this repo external resolution uses{ resolve: { external: true } }(seeapi-errors-refs.test.js). Using the wrong shape here can make--resolvenot actually follow external$refs.
api-linter/rulesets/functions/schema-test.js:11- This helper function prints full schemas to stdout. It looks like a debugging artifact, and if it ever gets enabled in a ruleset it will spam logs and slow runs. Consider removing the logs (or the whole file if unused).
const assertObjectSchema = schema => {
console.log('==========================================');
console.log(schema);
};
api-linter/test/api-security.test.js:16
- This test doesn't await/return
spectral.run(...), so assertions in the.then()may not run. Return/await the Promise here (and in the other tests in this file).
api-linter/test/api-errors.test.js:21 - Jest won't wait for this
.then()chain unless it's awaited/returned, so these assertions may never execute. Return/awaitspectral.run(...)here (and for the other tests in this file).
api-linter/test/api-sparse-fieldsets.test.js:15 - This test doesn't await/return the Promise returned by
spectral.run(...), which can cause Jest to pass the test without running assertions. Return/await the Promise here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copied from #66.
A linter tool with rulesets for developers following Siemens API Guidelines.