Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
106 changes: 106 additions & 0 deletions tools/spectral/ipa/__tests__/IPA112FieldNamesAreCamelCase.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import testRule from './__helpers__/testRule';
import { DiagnosticSeverity } from '@stoplight/types';

testRule('xgen-IPA-112-field-names-are-camel-case', [
{
name: 'valid schema - all field names are camelCase',
document: {
components: {
schemas: {
SchemaName: {
properties: {
userId: { type: 'string' },
firstName: { type: 'string' },
emailAddress: { type: 'string' },
phoneNumber: { type: 'string' },
},
},
},
},
},
errors: [],
},
{
name: 'invalid schema',
document: {
components: {
schemas: {
SchemaName: {
properties: {
UserId: { type: 'string' },
user_id: { type: 'string' },
'user-id': { type: 'string' },
},
},
},
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: Does it also catch nested schemas, and arrays etc.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • It should catch nested schemas. I can add a unit test to be sure.
  • What do you mean by catching arrays in this context? Can you give an example?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!

For arrays:

          ArraySchema: {
            type: 'array',
            items: {
              properties: {
                INVALID_FIELD: {},
              },
            },
          },

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean the properties in items. Sure, let me add unit tests covering both and see how it works

errors: [
{
code: 'xgen-IPA-112-field-names-are-camel-case',
message: 'Property "UserId" must use camelCase format.',
path: ['components', 'schemas', 'SchemaName', 'properties', 'UserId'],
severity: DiagnosticSeverity.Warning,
},
{
code: 'xgen-IPA-112-field-names-are-camel-case',
message: 'Property "user_id" must use camelCase format.',
path: ['components', 'schemas', 'SchemaName', 'properties', 'user_id'],
severity: DiagnosticSeverity.Warning,
},
{
code: 'xgen-IPA-112-field-names-are-camel-case',
message: 'Property "user-id" must use camelCase format.',
path: ['components', 'schemas', 'SchemaName', 'properties', 'user-id'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'schema with exception - non-camelCase field with exception',
document: {
components: {
schemas: {
SchemaName: {
properties: {
'NON-CAMEL-CASE': {
type: 'string',
'x-xgen-IPA-exception': {
'xgen-IPA-112-field-names-are-camel-case': 'Reason',
},
},
},
},
},
},
},
errors: [],
},
{
name: 'mixed valid, invalid, and exception fields',
document: {
components: {
schemas: {
SchemaName: {
properties: {
userId: { type: 'string' },
'API-Key': {
type: 'string',
'x-xgen-IPA-exception': {
'xgen-IPA-112-field-names-are-camel-case': 'Reason',
},
},
User_Name: {
type: 'string',
'x-xgen-IPA-exception': {
'xgen-IPA-112-field-names-are-camel-case': 'Reason',
},
},
},
},
},
},
},
errors: [],
},
]);
15 changes: 15 additions & 0 deletions tools/spectral/ipa/rulesets/IPA-112.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

functions:
- IPA112AvoidProjectFieldNames
- IPA112FieldNamesAreCamelCase

rules:
xgen-IPA-112-avoid-project-field-names:
Expand All @@ -29,3 +30,17 @@ rules:
alternative: ['groups']
ignore:
- 'gcp'
xgen-IPA-112-field-names-are-camel-case:
description: |
Schema field names should be in camelCase format.

##### Implementation details
Rule checks for the following conditions:
- Searches through all schemas in the API definition
- Identifies property names that are not in camelCase format
- Reports any instances where these field names are not in camelCase format
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-112-field-names-are-camel-case'
severity: warn
given: '$.components.schemas..properties[*]~'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: WDYT about also targeting inline schemas? Or should we only check component.schemas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great question! We should clarify what we mean by "field." To me, a field is a property of a resource schema, and resource schemas are typically defined under components.schemas. However, when I think about what I’d consider a resource schema, I’d primarily refer to response or requestBody content schemas as well.

So, are we specifically talking about inline schemas within those? Or, do we also count inline schemas used for query parameters? I’ve noticed that query parameters often use inline schemas, but they fall into a gray area—they're not path parameters, (the IPA for path params does not apply to them) and I’m not sure if their properties should be considered as fields

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think it could be valuable to catch fields in requests and responses as well, I'm working on the description IPA rule now and included them like so:

'$.paths[*][get,put,post,delete,options,head,patch,trace]..content..properties[*]'

In terms of query parameters, it's a good point. We don't talk a lot about them in the IPAs, but I'm thinking they should also be camelCase. Perhaps we can bring a new guideline for this. I think we can leave them out of this PR at least

then:
function: 'IPA112FieldNamesAreCamelCase'
11 changes: 11 additions & 0 deletions tools/spectral/ipa/rulesets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,17 @@ Rule checks for the following conditions:
- Reports any instances where these field names are used
- Suggests using "group", "groups", or "groupId" as alternatives

#### xgen-IPA-112-field-names-are-camel-case

![warn](https://img.shields.io/badge/warning-yellow)
Schema field names should be in camelCase format.

##### Implementation details
Rule checks for the following conditions:
- Searches through all schemas in the API definition
- Identifies property names that are not in camelCase format
- Reports any instances where these field names are not in camelCase format



### IPA-113
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { casing } from '@stoplight/spectral-functions';
import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
import { hasException } from './utils/exceptions.js';
import { resolveObject } from './utils/componentUtils.js';

const RULE_NAME = 'xgen-IPA-112-field-names-are-camel-case';

export default (input, options, { path, documentInventory }) => {
const oas = documentInventory.resolved;
const property = resolveObject(oas, path);

if (hasException(property, RULE_NAME)) {
collectException(property, RULE_NAME, path);
return;
}

if (casing(input, { type: 'camel', disallowDigits: true })) {
const errorMessage = `Property "${input}" must use camelCase format.`;
return collectAndReturnViolation(path, RULE_NAME, errorMessage);
}
collectAdoption(path, RULE_NAME);
};
Loading