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

feat(apidom-ls): lint parameter defined in path template #3571

Merged
merged 13 commits into from
Jan 4, 2024

Conversation

kowalczyk-krzysztof
Copy link
Member

@kowalczyk-krzysztof kowalczyk-krzysztof commented Dec 22, 2023

Refs #3546

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from 44242ea to 8c5af8c Compare December 22, 2023 10:56
@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from 8bfbe03 to 6e47f3f Compare January 1, 2024 00:39
@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from 6e47f3f to edfb653 Compare January 1, 2024 21:14
@char0n
Copy link
Member

char0n commented Jan 2, 2024

All right, so our first order of business if to determine if the Parameter Object we're looking at is somehow related to Path Item Object that is attached inside the Paths Object. Let's focus on that first, and after we have this established we'll focus on looking at Operation Object parameters.

Path Item Object parameters

We're specifically talking about PathItem.parameters.

1. Determining Path Item relationship

We first need to determine if Parameter Object is part of PathItem.parameters. That is achieved by looking at Parameter Object parent and checking if the parent is ArrayElement with meta class of path-item-parameters.

Predicate will look like this:

const isInPathItemElement = isArrayElement(paramerElement.parent) && includesClasses(['path-item-parameters'], parameterElement.parent)
const pathItemElement = parameterElement.parent.parent.parent

Tip

We can avoid step 1. and all the following steps by immediately looking into Parameter.in values and checking if it's path.

2. Determining if Path Item is part of path templating

To determine if the found Path Item Object (the one we found in previous step) is part of path templating, we have to look at it's meta field called path. If Path Item Object has this meta field and the meta field is of StringElement type, that we've determined that Parameter Object is part of path templating.

Predicate will look like this:

const isPathItemPartOfPathTemplating = isStringElement(pathItemElement.meta.get('path');
const pathTemplate = toValue(pathItemElement.meta.get('path'));

3. Determining if Parameter Object is matched with path templating

This is a bit tricky. We'll have to use openapi-path-templating to parse the path template into the AST:

This path template /pets/{petId} will parse into following AST:

          ['path-template', '/pets/{petId}'],
          ['path', '/pets/{petId}'],
          ['slash', '/'],
          ['path-literal', 'pets'],
          ['slash', '/'],
          ['template-expression', '{petId}'],
          ['template-expression-param-name', 'petId'],

If the Parameter.in === path - we'll have to look for template-expression-param-name with value of Parameter.name inside the AST.


Only after we have the above implementation, it makes sense to focus on Operation.parameters...

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from 13f39aa to 8918ce1 Compare January 2, 2024 15:02
@kowalczyk-krzysztof
Copy link
Member Author

kowalczyk-krzysztof commented Jan 2, 2024

Thanks, for the comment @char0n. I made some changes and it seems to be working fine for Operation.parameters but I'm still stuck on when the parameter is defined like this:

openapi: 3.0.0
info:
  title: Foo
  version: 0.1.0
components:
  parameters:
    test_id:
      name: test_id
      in: path
      required: true
      schema:
        type: string
        format: uuid
        title: Test Id

What do I do in this case? Did I misunderstand something?
EDIT: Indeed, I misunderstood.

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch 2 times, most recently from ccf0fcf to 0b25dd6 Compare January 2, 2024 17:37
@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from 0b25dd6 to 31fc3b3 Compare January 3, 2024 13:23
@kowalczyk-krzysztof kowalczyk-krzysztof marked this pull request as ready for review January 3, 2024 14:45
@char0n
Copy link
Member

char0n commented Jan 4, 2024

All right, so as discussed we'll handle the Operation.parameters in follow-up PR.

@kowalczyk-krzysztof kowalczyk-krzysztof force-pushed the parameter-defined-within-path-template branch from bb0c8f2 to 4d2dc74 Compare January 4, 2024 08:58
@kowalczyk-krzysztof
Copy link
Member Author

@char0n Thanks for the suggestions. I implemented all of them and I also updated fixtures to include all the possible places where Parameters object can be defined.

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

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

Looking good here. Nice job!

@char0n char0n merged commit 2ab2840 into main Jan 4, 2024
8 checks passed
@char0n char0n deleted the parameter-defined-within-path-template branch January 4, 2024 09:22
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.

None yet

2 participants