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

fix(apidom-ls): add deprecated warning for example property #2909

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

kowalczyk-krzysztof
Copy link
Member

Refs #2872

I'm not sure whether I understood the folder structure properly and if the code is in the right place. I'm also not sure which test file should be changed. I would appreciate some suggestions/help @char0n

@frantuma
Copy link
Member

frantuma commented Jul 5, 2023

@kowalczyk-krzysztof

Schema Object rules are defined in it's own package, under common.

A rule with a warning (possibly better warning than info?) would look like:

import { DiagnosticSeverity } from 'vscode-languageserver-types';

import ApilintCodes from '../../../codes';
import { LinterMeta } from '../../../../apidom-language-types';

const exampleDeprecatedLint: LinterMeta = {
  code: ApilintCodes.SCHEMA_EXAMPLE_DEPRECATED,
  source: 'apilint',
  message: 'property "example" is deprecated, use "examples" instead',
  severity: DiagnosticSeverity.Warning,
  linterFunction: 'missingField',
  linterParams: ['example'],
  marker: 'key',
  markerTarget: 'example',
  targetSpecs: [
    { namespace: 'openapi', version: '3.1.0' },
  ],
  data: {
    quickFix: [
      {
        message: 'remove example',
        action: 'removeChild',
        functionParams: ['example'],
        target: 'parent',
      },
      {
        message: "add 'examples' field",
        action: 'addChild',
        snippetYaml: 'examples: \n  ',
        snippetJson: '"examples": ,\n    ',
      },
    ],
  },
};

The sample code includes targetSpecs to specify to apply the rules only for OAS 3.1 (as schema is currently "shared" also with OAS 3.0 and AsyncAPI.
The sample also includes some possible quick fixes in case.

The current standard way to add rules to apidom-ls "core" is to organize them within the mentioned structure, just adding that it is also possible to add rules with a given field external to the dir structure, see e.g. this test. This can be useful to add custom rules in a client env or to do some testing.

In terms of tests, current status is "sub-optimal" for apidom-ls package, tests are not well designed/organized.

That said, an option is adding a test e.g. here (currently it compares stringified diagnostics result with hardcoded string).

for future ref, slightly related to #2481

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the fix/example-property-deprecated-in-oas31 branch from 5167294 to 08c9921 Compare July 6, 2023 06:23
@kowalczyk-krzysztof
Copy link
Member Author

@frantuma Thank you. I pushed the recommended changes and added test.

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the fix/example-property-deprecated-in-oas31 branch from c67fad1 to 72b405b Compare July 6, 2023 06:34
@kowalczyk-krzysztof kowalczyk-krzysztof changed the title fix(apidom-ls): add deprecated info for example property fix(apidom-ls): add deprecated warning for example property Jul 6, 2023
@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the fix/example-property-deprecated-in-oas31 branch 3 times, most recently from 28a446e to 2fccfd5 Compare July 6, 2023 07:30
@@ -0,0 +1,34 @@
import { DiagnosticSeverity } from 'vscode-languageserver-types';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved to src/config/common/schema/lint

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I did that already. My bad. Fixed.

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the fix/example-property-deprecated-in-oas31 branch from dfff274 to e629423 Compare July 6, 2023 11:24
@kowalczyk-krzysztof
Copy link
Member Author

The push after approval is just squashing two commits into one, as for some reason Lint commit would respond with:

header must not be longer than 69 characters, current length is 70 [header-max-length]

Even though the actual length was 40 characters.
Same issue happened when I created a merge commit to merge main into my branch - which I fixed by rebasing instead of merging.

@kowalczyk-krzysztof kowalczyk-krzysztof merged commit 564d185 into main Jul 6, 2023
7 checks passed
@kowalczyk-krzysztof kowalczyk-krzysztof deleted the fix/example-property-deprecated-in-oas31 branch July 6, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ApiDOM bug Something isn't working OpenAPI 3.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants