Skip to content

Commit

Permalink
feat: added linter rule
Browse files Browse the repository at this point in the history
Signed-off-by: Tokesh <tokesh789@gmail.com>
  • Loading branch information
Tokesh committed Feb 10, 2025
1 parent 2818751 commit fd72587
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 61 deletions.
8 changes: 0 additions & 8 deletions .github/vale/styles/OpenSearch/NumericFormat.yml

This file was deleted.

1 change: 0 additions & 1 deletion .vale.ini
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ OpenSearch.LoginNoun = YES
OpenSearch.LoginVerb = YES
OpenSearch.LogoutNoun = YES
OpenSearch.LogoutVerb = YES
OpenSearch.NumericFormat = YES
OpenSearch.OxfordComma = YES
OpenSearch.PassiveVoice = NO
OpenSearch.Please = YES
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +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 vale rule for numeric types ([#800](https://github.com/opensearch-project/opensearch-api-specification/pull/800))
- 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
1 change: 0 additions & 1 deletion spec/namespaces/_core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2273,7 +2273,6 @@ components:
- $ref: '../schemas/_core.bulk.yaml#/components/schemas/OperationContainer'
- $ref: '../schemas/_core.bulk.yaml#/components/schemas/UpdateAction'
- type: object
- type: number
description: The operation definition and data (action-data pairs), separated by newlines
required: true
bulk_stream:
Expand Down
15 changes: 0 additions & 15 deletions test.yml

This file was deleted.

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 @@ import {
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 @@ export default class SchemaVisitingValidator {
return errors
}

validateFile(filePath: string): ValidationError[] {

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

View workflow job for this annotation

GitHub Actions / lint

Class Method name `validateFile` must match one of the following formats: snake_case

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

View workflow job for this annotation

GitHub Actions / lint

Parameter name `filePath` must match one of the following formats: snake_case, UPPER_CASE
const errors: ValidationError[] = []
const visitor = this.createVisitor(errors)

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

View workflow job for this annotation

GitHub Actions / lint

Trailing spaces not allowed
const targetFile = [...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

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

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

visitor.visit_specification(new SpecificationContext(targetFile.file), targetFile.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 @@ export default class SchemaVisitingValidator {
}

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 InfoFile from './components/InfoFile'
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 @@ export default class SpecValidator {
schemas_validator: SchemasValidator
schema_refs_validator: SchemaRefsValidator
inline_object_schema_validator: SchemaVisitingValidator
changedOnly: boolean

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

View workflow job for this annotation

GitHub Actions / lint

Class Property name `changedOnly` must match one of the following formats: snake_case

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 @@ export default class SpecValidator {
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.changedOnly = 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:' + error)

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

View workflow job for this annotation

GitHub Actions / lint

Invalid operand for a '+' operation. Operands must each be a number or string, allowing a string + any of: `any`, `boolean`, `null`, `RegExp`, `undefined`. Got `unknown`
return []
}
}

validateChangedFiles(): ValidationError[] {

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

View workflow job for this annotation

GitHub Actions / lint

Class Method name `validateChangedFiles` must match one of the following formats: snake_case
const changedFiles = this.getChangedFiles()

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

View workflow job for this annotation

GitHub Actions / lint

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

if (changedFiles.length === 0) {
this.logger.log('No valid files to check')
return []
}

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

for (const file of changedFiles) {
let normalizedFile = file

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

errors = errors.concat(this.inline_object_schema_validator.validateFile(normalizedFile))
}

return errors
}

validate (): ValidationError[] {
if (this.changedOnly) return this.validateChangedFiles()

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
25 changes: 8 additions & 17 deletions tools/src/prepare-for-vale/KeepDescriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import fg from 'fast-glob'
import { Logger } from '../Logger'

/**
* Keeps only description: field values and type: number with its next line.
* Keeps only description: field values.
*/
export default class KeepDescriptions {
root_folder: string
Expand All @@ -34,37 +34,28 @@ export default class KeepDescriptions {

process_file(filename: string): void {
const contents = fs.readFileSync(filename, 'utf-8')
const lines = contents.split(/\r?\n/)
const writer = fs.openSync(filename, 'w+')

let inside_text = false
for (let i = 0; i < lines.length; i++) {
let line = lines[i]
var writer = fs.openSync(filename, 'w+')

var inside_text = false
contents.split(/\r?\n/).forEach((line) => {
if (line.match(/^[\s]+((description|x-deprecation-message): \|)/)) {
inside_text = true
} else if (line.match(/^[\s]+((description|x-deprecation-message):)[\s]+/)) {
let cleaned_line = this.prune(line, /(description|x-deprecation-message):/, ' ')
cleaned_line = this.prune_vars(cleaned_line)
cleaned_line = this.remove_links(cleaned_line)
fs.writeSync(writer, cleaned_line + "\n")
fs.writeSync(writer, cleaned_line)
} else if (inside_text && line.match(/^[\s]*[\w\\$]*:/)) {
inside_text = false
} else if (inside_text) {
let cleaned_line = this.remove_links(line)
cleaned_line = this.prune_vars(cleaned_line)
fs.writeSync(writer, cleaned_line + "\n")
} else if (line.includes("type: number")) {
fs.writeSync(writer, line + "\n")
if (i + 1 < lines.length) {
fs.writeSync(writer, lines[i + 1] + "\n")
}
fs.writeSync(writer, cleaned_line)
}

if (line.length > 0) {
fs.writeSync(writer, "\n")
}
}
})
}

prune_vars(line: string): string {
Expand All @@ -83,4 +74,4 @@ export default class KeepDescriptions {
return ' ' + p1 + spaces
})
}
}
}

0 comments on commit fd72587

Please sign in to comment.