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.

Sign up for GitHub

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

Jump to bottom

Fixed Linting for Tools #278

Merged
merged 1 commit into from
May 2, 2024
Merged

Fixed Linting for Tools #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 38 additions & 19 deletions tools/eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,45 @@ export default [
...compat.extends('standard-with-typescript'),
{
files: ['**/*.{js,ts}'],
// to auto-fix disable all rules except the one you want to fix with '@rule': 'warn', then run `npm run lint -- --fix`
rules: {
'@typescript-eslint/consistent-indexed-object-style': 'warn',
'@typescript-eslint/consistent-type-assertions': 'warn',
'@typescript-eslint/dot-notation': 'warn',
'@typescript-eslint/explicit-function-return-type': 'warn',
'@typescript-eslint/naming-convention': 'warn',
'@typescript-eslint/no-confusing-void-expression': 'warn',
'@typescript-eslint/no-dynamic-delete': 'warn',
'@typescript-eslint/no-invalid-void-type': 'warn',
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/no-unnecessary-type-assertion': 'warn',
'@typescript-eslint/no-unsafe-argument': 'warn',
'@typescript-eslint/prefer-nullish-coalescing': 'warn',
'@typescript-eslint/require-array-sort-compare': 'warn',
'@typescript-eslint/strict-boolean-expressions': 'warn',
'array-callback-return': 'warn',
'new-cap': 'warn',
'no-return-assign': 'warn',
'object-shorthand': 'warn'
'@typescript-eslint/consistent-indexed-object-style': 'error',
'@typescript-eslint/consistent-type-assertions': 'error',
'@typescript-eslint/dot-notation': 'error',
'@typescript-eslint/explicit-function-return-type': 'error',
'@typescript-eslint/naming-convention': ['error',
{ selector: 'classProperty', modifiers: ['readonly'], format: ['UPPER_CASE'], leadingUnderscore: 'allow' },
{ selector: 'memberLike', modifiers: ['public'], format: ['snake_case'], leadingUnderscore: 'forbid' },
{ selector: 'memberLike', modifiers: ['private', 'protected'], format: ['snake_case'], leadingUnderscore: 'require' },
{ selector: 'variableLike', format: ['snake_case', 'UPPER_CASE'], leadingUnderscore: 'allow' },
{ selector: 'typeLike', format: ['PascalCase'] },
{ selector: 'objectLiteralProperty', format: null },
{ selector: 'typeProperty', format: null }
],
'@typescript-eslint/no-confusing-void-expression': 'error',
'@typescript-eslint/no-dynamic-delete': 'error',
'@typescript-eslint/no-invalid-void-type': 'error',
'@typescript-eslint/no-non-null-assertion': 'error',
'@typescript-eslint/no-unnecessary-type-assertion': 'error',
'@typescript-eslint/no-unsafe-argument': 'error',
'@typescript-eslint/prefer-nullish-coalescing': 'error',
'@typescript-eslint/require-array-sort-compare': 'error',
'@typescript-eslint/strict-boolean-expressions': ['error',
{
allowString: true,
allowNumber: true,
allowNullableObject: true,
allowNullableBoolean: true,
allowNullableString: false,
allowNullableNumber: false,
allowNullableEnum: false,
allowAny: false,
allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: false
}
],
'array-callback-return': 'off',
'new-cap': 'off',
Comment on lines +51 to +52
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we disable these lints?

Copy link
Collaborator Author

@nhtruong nhtruong May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Both of these are very low risk, IMO.

  • The array-callback-return requires all paths of a map function to return something. We have a lot of map calls that either return a ValidationError for a member of the iterable causes a lint error, or return nothing if no infractions are found. So the end results is an array with a mix of ValidationErrors and undefined. We then filter out the undefined to get a list of ValidationErrors. In other words, we intentional want undefined in the array returned by map. That rule forces the function passed to map to explicitly have a return line for the undefined case (but not other kinds of functions). I don't see much benefit of it. This seems very unique to eslint.
  • new-cap requires that when you use the new keyword, the next word must be capitalized. const validator = (new ajv()).compile(schema) would be flagged because ajv is not capitalized even though we just imported ajv as is.

'no-return-assign': 'error',
'object-shorthand': 'error'
}
}
]
27 changes: 14 additions & 13 deletions tools/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import fs from 'fs'
import YAML from 'yaml'
import _ from 'lodash'

export function resolveRef (ref: string, root: Record<string, any>): Record<string, any> | undefined {
export function resolve_ref (ref: string, root: Record<string, any>): Record<string, any> | undefined {
const paths = ref.replace('#/', '').split('/')
for (const p of paths) {
root = root[p]
Expand All @@ -11,42 +11,43 @@ export function resolveRef (ref: string, root: Record<string, any>): Record<stri
return root
}

export function resolveObj (obj: Record<string, any> | undefined, root: Record<string, any>) {
export function resolve_obj (obj: Record<string, any> | undefined, root: Record<string, any>): Record<string, any> | undefined {
if (obj === undefined) return undefined
if (obj.$ref) return resolveRef(obj.$ref, root)
if (obj.$ref !== null) return resolve_ref(obj.$ref as string, root)
return obj
}

export function dig (obj: Record<string, any>, path: string[], root: Record<string, any>): any {
let value = obj
for (const p of path) {
value = resolveObj(value, root)?.[p]
value = resolve_obj(value, root)?.[p]
if (value === undefined) break
}
return value
}

export function sortByKey (obj: Record<string, any>, priorities: string[] = []) {
export function sort_by_keys (obj: Record<string, any>, priorities: string[] = []): void {
const orders = _.fromPairs(priorities.map((k, i) => [k, i + 1]))
const sorted = _.entries(obj).sort((a, b) => {
const order_a = orders[a[0]]
const order_b = orders[b[0]]
if (order_a && order_b) return order_a - order_b
if (order_a) return 1
if (order_b) return -1
if (order_a != null && order_b != null) return order_a - order_b
if (order_a != null) return 1
if (order_b != null) return -1
return a[0].localeCompare(b[0])
})
sorted.forEach(([k, v]) => {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete obj[k]
obj[k] = v
})
}

export function write2file (file_path: string, content: Record<string, any>): void {
fs.writeFileSync(file_path, quoteRefs(YAML.stringify(removeAnchors(content), { lineWidth: 0, singleQuote: true })))
export function write_to_yaml (file_path: string, content: Record<string, any>): void {
fs.writeFileSync(file_path, quote_refs(YAML.stringify(remove_anchors(content), { lineWidth: 0, singleQuote: true })))
}

function quoteRefs (str: string): string {
function quote_refs (str: string): string {
return str.split('\n').map((line) => {
if (line.includes('$ref')) {
const [key, value] = line.split(': ')
Expand All @@ -56,7 +57,7 @@ function quoteRefs (str: string): string {
}).join('\n')
}

function removeAnchors (content: Record<string, any>): Record<string, any> {
const replacer = (key: string, value: any) => key === '$anchor' ? undefined : value
function remove_anchors (content: Record<string, any>): Record<string, any> {
const replacer = (key: string, value: any): any => key === '$anchor' ? undefined : value
return JSON.parse(JSON.stringify(content, replacer))
}
17 changes: 9 additions & 8 deletions tools/linter/PathRefsValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@ export default class PathRefsValidator {
this.#build_available_paths()
}

#build_referenced_paths () {
#build_referenced_paths (): void {
for (const [path, spec] of Object.entries(this.root_file.spec().paths)) {
const ref = spec!.$ref!
const ref = spec?.$ref ?? ''
const file = ref.split('#')[0]
if (!this.referenced_paths[file]) this.referenced_paths[file] = new Set()
const ref_path = this.referenced_paths[file] as Set<string> | undefined
if (!ref_path) this.referenced_paths[file] = new Set()
this.referenced_paths[file].add(path)
}
}

#build_available_paths () {
#build_available_paths (): void {
for (const file of this.namespaces_folder.files) {
this.available_paths[file.file] = new Set(Object.keys(file.spec().paths || {}))
this.available_paths[file.file] = new Set(Object.keys(file.spec().paths ?? {}))
}
}

Expand All @@ -40,7 +41,7 @@ export default class PathRefsValidator {

validate_unresolved_refs (): ValidationError[] {
return Object.entries(this.referenced_paths).flatMap(([ref_file, ref_paths]) => {
const available = this.available_paths[ref_file]
const available = this.available_paths[ref_file] as Set<string> | undefined
if (!available) {
return {
file: this.root_file.file,
Expand All @@ -63,15 +64,15 @@ export default class PathRefsValidator {

validate_unreferenced_paths (): ValidationError[] {
return Object.entries(this.available_paths).flatMap(([ns_file, ns_paths]) => {
const referenced = this.referenced_paths[ns_file]
const referenced = this.referenced_paths[ns_file] as Set<string> | undefined
if (!referenced) {
return {
file: ns_file,
message: 'Unreferenced paths: No paths are referenced in the root file.'
}
}
return Array.from(ns_paths).map((path) => {
if (!referenced || !referenced.has(path)) {
if (!referenced?.has(path)) {
return {
file: ns_file,
location: `Path: ${path}`,
Expand Down
42 changes: 22 additions & 20 deletions tools/linter/SchemaRefsValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,41 @@ export default class SchemaRefsValidator {
this.#build_available_schemas()
}

#find_refs_in_namespaces_folder () {
const search = (obj: Record<string, any>) => {
const ref = obj.$ref
if (ref) {
#find_refs_in_namespaces_folder (): void {
const search = (obj: any): void => {
const ref: string = obj.$ref ?? ''
if (ref !== '') {
const file = ref.split('#')[0].replace('../', '')
const name = ref.split('/').pop()
if (!this.referenced_schemas[file]) this.referenced_schemas[file] = new Set()
const name = ref.split('/').pop() ?? ''
if (name === '') throw new Error(`Invalid schema reference: ${ref}`)
if (this.referenced_schemas[file] == null) this.referenced_schemas[file] = new Set()
this.referenced_schemas[file].add(name)
}
for (const key in obj) { if (typeof obj[key] === 'object') search(obj[key]) }
}

this.namespaces_folder.files.forEach((file) => { search(file.spec().components || {}) })
this.namespaces_folder.files.forEach((file) => { search(file.spec().components ?? {}) })
}

#find_refs_in_schemas_folder () {
const search = (obj: Record<string, any>, ref_file: string) => {
const ref = obj.$ref
if (ref) {
#find_refs_in_schemas_folder (): void {
const search = (obj: any, ref_file: string): void => {
const ref = obj.$ref as string ?? ''
if (ref !== '') {
const file = ref.startsWith('#') ? ref_file : `schemas/${ref.split('#')[0]}`
const name = ref.split('/').pop()
if (!this.referenced_schemas[file]) this.referenced_schemas[file] = new Set()
const name = ref.split('/').pop() ?? ''
if (name === '') throw new Error(`Invalid schema reference: ${ref}`)
if (this.referenced_schemas[file] == null) this.referenced_schemas[file] = new Set()
this.referenced_schemas[file].add(name)
}
for (const key in obj) { if (typeof obj[key] === 'object') search(obj[key], ref_file) }
}

this.schemas_folder.files.forEach((file) => { search(file.spec().components?.schemas || {}, file.file) })
this.schemas_folder.files.forEach((file) => { search(file.spec().components?.schemas ?? {}, file.file) })
}

#build_available_schemas () {
#build_available_schemas (): void {
this.schemas_folder.files.forEach((file) => {
this.available_schemas[file.file] = new Set(Object.keys(file.spec().components?.schemas || {}))
this.available_schemas[file.file] = new Set(Object.keys(file.spec().components?.schemas ?? {}))
})
}

Expand All @@ -63,7 +65,7 @@ export default class SchemaRefsValidator {
validate_unresolved_refs (): ValidationError[] {
return Object.entries(this.referenced_schemas).flatMap(([ref_file, ref_schemas]) => {
const available = this.available_schemas[ref_file]
if (!available) {
if (available == null) {
return {
file: this.namespaces_folder.file,
message: `Unresolved schema reference: Schema file ${ref_file} is referenced but does not exist.`
Expand All @@ -85,17 +87,17 @@ export default class SchemaRefsValidator {
validate_unreferenced_schemas (): ValidationError[] {
return Object.entries(this.available_schemas).flatMap(([file, schemas]) => {
const referenced = this.referenced_schemas[file]
if (!referenced) {
if (referenced == null) {
return {
file: file,
file,
message: `Unreferenced schema: Schema file ${file} is not referenced anywhere.`
}
}

return Array.from(schemas).map((schema) => {
if (!referenced.has(schema)) {
return {
file: file,
file,
location: `#/components/schemas/${schema}`,
message: `Unreferenced schema: Schema ${schema} is not referenced anywhere.`
}
Expand Down
28 changes: 15 additions & 13 deletions tools/linter/components/NamespaceFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ import { type OperationSpec, type ValidationError } from '../../types'
import OperationGroup from './OperationGroup'
import _ from 'lodash'
import Operation from './Operation'
import { resolveRef } from '../../helpers'
import { resolve_ref } from '../../helpers'
import FileValidator from './base/FileValidator'

const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace']
const NAME_REGEX = /^[a-z]+[a-z_]*[a-z]+$/

export default class NamespaceFile extends FileValidator {
namespace: string
_operation_groups: OperationGroup[] | undefined
_refs: Set<string> | undefined
private _operation_groups: OperationGroup[] | undefined
private _refs: Set<string> | undefined

constructor (file_path: string) {
super(file_path)
Expand Down Expand Up @@ -41,47 +41,49 @@ export default class NamespaceFile extends FileValidator {
})
})

return this._operation_groups = _.entries(_.groupBy(ops, (op) => op.group)).map(([group, ops]) => {
this._operation_groups = _.entries(_.groupBy(ops, (op) => op.group)).map(([group, ops]) => {
return new OperationGroup(this.file, group, ops)
})
return this._operation_groups
}

refs (): Set<string> {
if (this._refs) return this._refs
this._refs = new Set<string>()
const find_refs = (obj: Record<string, any>) => {
if (obj.$ref) this._refs!.add(obj.$ref)
_.values(obj).forEach((value) => { if (typeof value === 'object') find_refs(value) })
const find_refs = (obj: Record<string, any>): void => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
if (obj.$ref != null) this._refs!.add(obj.$ref as string)
_.values(obj).forEach((value) => { if (typeof value === 'object') find_refs(value as Record<string, any>) })
}
find_refs(this.spec().paths || {})
find_refs(this.spec().paths ?? {})
return this._refs
}

validate_name (name = this.namespace): ValidationError | void {
validate_name (name = this.namespace): ValidationError | undefined {
if (name === '_core') return
if (!name.match(NAME_REGEX)) { return this.error(`Invalid namespace name '${name}'. Must match regex: /${NAME_REGEX.source}/.`, 'File Name') }
}

validate_schemas (): ValidationError | void {
validate_schemas (): ValidationError | undefined {
if (this.spec().components?.schemas) { return this.error('components/schemas is not allowed in namespace files', '#/components/schemas') }
}

validate_unresolved_refs (): ValidationError[] {
return Array.from(this.refs()).map((ref) => {
if (resolveRef(ref, this.spec()) === undefined) return this.error(`Unresolved reference: ${ref}`, ref)
if (resolve_ref(ref, this.spec()) === undefined) return this.error(`Unresolved reference: ${ref}`, ref)
}).filter((e) => e) as ValidationError[]
}

validate_unused_refs (): ValidationError[] {
return _.entries(this.spec().components || {}).flatMap(([type, collection]) => {
return _.entries(this.spec().components ?? {}).flatMap(([type, collection]) => {
return _.keys(collection).map((name) => {
if (!this.refs().has(`#/components/${type}/${name}`)) { return this.error(`Unused ${type} component: ${name}`, `#/components/${type}/${name}`) }
})
}).filter((e) => e) as ValidationError[]
}

validate_parameter_refs (): ValidationError[] {
const parameters = this.spec().components?.parameters as Record<string, OpenAPIV3.ParameterObject>
const parameters = this.spec().components?.parameters as Record<string, OpenAPIV3.ParameterObject> | undefined
if (!parameters) return []
return _.entries(parameters).map(([name, p]) => {
const group = name.split('::')[0]
Expand Down
8 changes: 4 additions & 4 deletions tools/linter/components/NamespacesFolder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ export default class NamespacesFolder extends FolderValidator<NamespaceFile> {
}

validate_duplicate_paths (): ValidationError[] {
const paths: { [path: string]: string[] } = {}
const paths: Record<string, string[]> = {}
for (const file of this.files) {
if (!file._spec?.paths) continue
if (file.spec().paths == null) continue
Object.keys(file.spec().paths).sort().forEach((path) => {
if (paths[path]) paths[path].push(file.namespace)
else paths[path] = [file.namespace]
if (paths[path] == null) paths[path] = [file.namespace]
else paths[path].push(file.namespace)
})
}
return Object.entries(paths).map(([path, namespaces]) => {
Expand Down
Loading
Loading