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(ls): process schema 'null' / 'nullable' depending on spec version #2982

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

frantuma
Copy link
Member

Differentiates behavior for type null validation/completion depending on spec/version, also add validation rule for nullable adding hint for all asyncapi and openapi spec versions except openapi 3.0.x. AsyncAPI also included as users can typically come from openapi 3.0 when designing an AsyncAPI spec

@@ -56,6 +57,7 @@ import thenNonIfLint from './then--non-if';
import thenTypeLint from './then--type';
import titleTypeLint from './title--type';
import typeTypeLint from './type--type';
import typeTypeLintOpenApi30 from './type--type-openapi-3-0';
Copy link
Member

Choose a reason for hiding this comment

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

Just consistent naming with the rest of config

Suggested change
import typeTypeLintOpenApi30 from './type--type-openapi-3-0';
import typeTypeOpenAPI3_0Lint from './type--type-openapi-3-0';

Copy link
Member Author

Choose a reason for hiding this comment

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

@char0n
this doesn't seem to be liked by linter:

Variable name typeTypeOpenAPI3_0Lint must match one of the following formats: camelCase, PascalCase, UPPER_CASE

Copy link
Member

Choose a reason for hiding this comment

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

Yep, we disable it inline as in

// eslint-disable-next-line @typescript-eslint/naming-convention

We use this convention for element names containing version numbers as well.

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

const nullableNotRecommendedLint: LinterMeta = {
code: ApilintCodes.SCHEMA_EXAMPLE_DEPRECATED,
Copy link
Member

Choose a reason for hiding this comment

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

new code needs to be created and assigned here

const nullableNotRecommendedLint: LinterMeta = {
code: ApilintCodes.SCHEMA_EXAMPLE_DEPRECATED,
source: 'apilint',
message: 'nullable has no special meaning, if not set on purpose use "type=null" instead',
Copy link
Member

Choose a reason for hiding this comment

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

value of type must always be string even in case of the null type. This will avoid some confusion that this message might trigger

Suggested change
message: 'nullable has no special meaning, if not set on purpose use "type=null" instead',
message: 'nullable has no special meaning, if not set on purpose use `type="null"` instead',

@@ -120,6 +123,7 @@ const schemaLints = [
thenTypeLint,
titleTypeLint,
typeTypeLint,
typeTypeLintOpenApi30,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
typeTypeLintOpenApi30,
typeTypeOpenAPI3_0Lint,

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

const typeTypeLintOpenApi30: LinterMeta = {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const typeTypeLintOpenApi30: LinterMeta = {
const typeTypeOpenAPI3_0Lint: LinterMeta = {

@frantuma frantuma merged commit d053dca into main Jul 25, 2023
@frantuma frantuma deleted the fix-null-nullable branch July 25, 2023 13:26
frankkilcommins pushed a commit to frankkilcommins/apidom that referenced this pull request Sep 19, 2023
Error code duplicates were uncovered by eslint related 
libs update.

Closes swagger-api#2982
Closes swagger-api#2923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants