Skip to content

Added a linter for numeric types #800

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

Closed
wants to merge 14 commits into from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `GET`, `POST /_plugins/_ml/tasks/_search`, `GET /_plugins/_ml/tools`, `tools/{tool_name}` ([#797](https://github.com/opensearch-project/opensearch-api-specification/pull/797))
- Added `POST /_plugins/_ml/agents/{agent_id}/_execute`, `GET /_plugins/_ml/agents/{agent_id}`, `GET`, `POST /_plugins/_ml/agents/_search` ([#798](https://github.com/opensearch-project/opensearch-api-specification/pull/798))
- Added a warning for test file names that don't match the API being tested ([#793](https://github.com/opensearch-project/opensearch-api-specification/pull/793))
- Added a linter rule for numeric types ([#800](https://github.com/opensearch-project/opensearch-api-specification/pull/800))
- Added `time` field to the `GetStats` schema in `_common.yml` ([#803](https://github.com/opensearch-project/opensearch-api-specification/pull/803))
- Added version for `POST /_plugins/_ml/_train/{algorithm_name}`, `_predict/{algorithm_name}/{model_id}`, and `_train_predict/{algorithm_name}` ([#763](https://github.com/opensearch-project/opensearch-api-specification/pull/763))
- Added `POST _plugins/_security/api/internalusers/{username}` response `201` ([#810](https://github.com/opensearch-project/opensearch-api-specification/pull/810))
Expand Down
50 changes: 34 additions & 16 deletions tools/src/linter/SchemaVisitingValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
SpecificationContext
} from '../_utils'
import { type OpenAPIV3 } from 'openapi-types'
import { namespace_file } from '../../tests/linter/factories/namespace_file'

Check failure on line 23 in tools/src/linter/SchemaVisitingValidator.ts

View workflow job for this annotation

GitHub Actions / lint

'namespace_file' is defined but never used

export default class SchemaVisitingValidator {
private readonly _namespaces_folder: NamespacesFolder
Expand Down Expand Up @@ -52,6 +53,33 @@
return errors
}

validate_file(file_path: string): ValidationError[] {
const errors: ValidationError[] = []
const visitor = this.createVisitor(errors)

const target_file = [...this._namespaces_folder.files, ...this._schemas_folder.files].find(f => f.file === filePath)

Check failure on line 60 in tools/src/linter/SchemaVisitingValidator.ts

View workflow job for this annotation

GitHub Actions / lint

'filePath' is not defined

Check failure on line 60 in tools/src/linter/SchemaVisitingValidator.ts

View workflow job for this annotation

GitHub Actions / validate

Cannot find name 'filePath'. Did you mean 'file_path'?

if (!target_file) {
return [{ message: `File not found in namespaces/schemas: ${file_path}`, file: file_path }]
}

visitor.visit_specification(new SpecificationContext(target_file.file), target_file.spec())

return errors
}

private createVisitor(errors: ValidationError[]): SchemaVisitor {
const validating_functions = [
this.#validate_numeric_schema.bind(this)
]

return new SchemaVisitor((ctx, schema) => {
for (const f of validating_functions) {
f(ctx, schema, errors)
}
})
}

#validate_inline_object_schema (ctx: SpecificationContext, schema: MaybeRef<OpenAPIV3.SchemaObject>, errors: ValidationError[]): void {
if (is_ref(schema) || schema.type !== 'object' || schema.properties === undefined) {
return
Expand All @@ -73,26 +101,16 @@
}

if (schema.type === 'number') {
if (schema.format === undefined || SCHEMA_NUMBER_FORMATS.includes(schema.format)) {
return
}

if (SCHEMA_INTEGER_FORMATS.includes(schema.format)) {
errors.push(ctx.error(`schema of type 'number' with format '${schema.format}' should instead be of type 'integer'`))
} else {
errors.push(ctx.error(`schema of type 'number' with format '${schema.format}' is invalid, expected one of: ${SCHEMA_NUMBER_FORMATS.join(', ')}`))
if (schema.format == null || !schema.format) {
errors.push(ctx.error(`Schema of type 'number' must specify a valid format. Allowed formats: ${SCHEMA_NUMBER_FORMATS.join(', ')}`));
return;
}
}

if (schema.type === 'integer') {
if (schema.format === undefined || SCHEMA_INTEGER_FORMATS.includes(schema.format)) {
return
}

if (SCHEMA_NUMBER_FORMATS.includes(schema.format)) {
errors.push(ctx.error(`schema of type 'integer' with format '${schema.format}' should instead be of type 'number'`))
} else {
errors.push(ctx.error(`schema of type 'integer' with format '${schema.format}' is invalid, expected one of: ${SCHEMA_INTEGER_FORMATS.join(', ')}`))
if (schema.format == null || !schema.format) {
errors.push(ctx.error(`Schema of type 'integer' must specify a valid format. Allowed formats: ${SCHEMA_INTEGER_FORMATS.join(', ')}`));
return;
}
}
}
Expand Down
44 changes: 43 additions & 1 deletion tools/src/linter/SpecValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import SchemaVisitingValidator from './SchemaVisitingValidator'
import SchemasValidator from './SchemasValidator'
import { type Logger } from '../Logger'
import { execSync } from 'child_process'
import { relative, resolve } from 'path'

export default class SpecValidator {
logger: Logger
Expand All @@ -26,8 +28,9 @@
schemas_validator: SchemasValidator
schema_refs_validator: SchemaRefsValidator
inline_object_schema_validator: SchemaVisitingValidator
changed_only: boolean

constructor (root_folder: string, logger: Logger) {
constructor (root_folder: string, logger: Logger, changedOnly: boolean) {

Check failure on line 33 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Parameter name `changedOnly` must match one of the following formats: snake_case, UPPER_CASE
this.logger = logger
this.superseded_ops_file = new SupersededOperationsFile(`${root_folder}/_superseded_operations.yaml`)
this.info_file = new InfoFile(`${root_folder}/_info.yaml`)
Expand All @@ -36,9 +39,48 @@
this.schemas_validator = new SchemasValidator(root_folder, logger)
this.schema_refs_validator = new SchemaRefsValidator(this.namespaces_folder, this.schemas_folder)
this.inline_object_schema_validator = new SchemaVisitingValidator(this.namespaces_folder, this.schemas_folder)
this.changed_only = changedOnly
}

private getChangedFiles(): string[] {
try {
const output = execSync('git diff --name-only origin/main', { encoding: 'utf-8' })
return output.split('\n').filter(file => file.endsWith('.yml') || file.endsWith('.yaml'))
} catch (error) {
this.logger.error(`Error getting changed files: ${error}`)

Check failure on line 50 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Invalid type "unknown" of template literal expression
return []
}
}

validate_changed_files(): ValidationError[] {
const changed_files = this.getChangedFiles()

Check failure on line 57 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
if (changed_files.length === 0) {
this.logger.log('No valid files to check')
return []
}

Check failure on line 62 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
this.logger.log(`Checking diff files:\n${changed_files.join('\n')}`)
let errors: ValidationError[] = []

Check failure on line 65 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
for (const file of changed_files) {
let normalizedFile = file

Check failure on line 67 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Variable name `normalizedFile` must match one of the following formats: snake_case, UPPER_CASE

Check failure on line 68 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
if (file.startsWith('spec/')) {
normalizedFile = relative(resolve('spec'), resolve(file))
} else if (file.startsWith('schemas/')) {
normalizedFile = relative(resolve('schemas'), resolve(file))
}

Check failure on line 74 in tools/src/linter/SpecValidator.ts

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
errors = errors.concat(this.inline_object_schema_validator.validate_file(normalizedFile))
}

return errors
}

validate (): ValidationError[] {
if (this.changed_only) return this.validate_changed_files()

const component_errors = [
...this.namespaces_folder.validate(),
...this.schemas_folder.validate()
Expand Down
3 changes: 2 additions & 1 deletion tools/src/linter/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ const command = new Command()
.description('Validate the OpenSearch multi-file spec.')
.addOption(new Option('-s, --source <path>', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec')))
.addOption(new Option('--verbose', 'whether to print the full stack trace of errors').default(false))
.addOption(new Option('--changed-only', 'whether to only validate changed files').default(false))
.allowExcessArguments(false)
.parse()

const opts = command.opts()
const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn)
logger.log(`Validating ${opts.source} ...`)
const validator = new SpecValidator(opts.source, logger)
const validator = new SpecValidator(opts.source, logger, opts.changedOnly)
const errors = validator.validate()

if (errors.length === 0) {
Expand Down
2 changes: 1 addition & 1 deletion tools/src/prepare-for-vale/KeepDescriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,4 @@ export default class KeepDescriptions {
return ' ' + p1 + spaces
})
}
}
}
Loading