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

fix(cli): Add validation rule for recursively-defined types (Issue #4135) #4224

Closed
wants to merge 1 commit into from

Conversation

Schnides123
Copy link
Contributor

Adds rule and related utilities for detecting & marking cyclicly-defined types.

@Schnides123 Schnides123 requested a review from dsinghvi as a code owner August 7, 2024 04:12
@Schnides123 Schnides123 changed the title Feature: Add validation rule for recursively-defined types [Draft] Feature: Add validation rule for recursively-defined types Aug 7, 2024
@Schnides123 Schnides123 changed the title [Draft] Feature: Add validation rule for recursively-defined types [Draft] Fix: Add validation rule for recursively-defined types (#4135) Aug 7, 2024
@Schnides123 Schnides123 changed the title [Draft] Fix: Add validation rule for recursively-defined types (#4135) Fix: Add validation rule for recursively-defined types (#4135) Aug 11, 2024
@Schnides123 Schnides123 force-pushed the main branch 2 times, most recently from 5069ed6 to c416e63 Compare August 11, 2024 06:54
@Schnides123 Schnides123 changed the title Fix: Add validation rule for recursively-defined types (#4135) Fix: Add validation rule for recursively-defined types (fern-api#4135) Aug 11, 2024
@Schnides123 Schnides123 changed the title Fix: Add validation rule for recursively-defined types (fern-api#4135) Fix: Add validation rule for recursively-defined types (Issue #4135) Aug 11, 2024
}

/**
* This function will return the first found path to a named child type from a given type. It will only search within the file of the given type.
Copy link
Member

Choose a reason for hiding this comment

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

It will only search within the file of the given type.

Seems like a big limitation, no?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/fern-api/fern/blob/main/packages/cli/yaml/validator/src/rules/no-duplicate-declarations/no-duplicate-declarations.ts#L15 Is an example of a rule that is able to check for name conflicts across files (admittedly much easier)

Copy link
Contributor Author

@Schnides123 Schnides123 Aug 19, 2024

Choose a reason for hiding this comment

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

It will only search within the file of the given type.

Seems like a big limitation, no?

Hey, thanks for taking a look at this. I had actually ran into that while coding, but we should be covered by the existing validation rule for cyclic imports.

ex1
The only way to create a cycle across multiple files is if each file has a reference to the type defined in the other file, as seen above. That would inherently require a cyclic import, so the validation would fail it and prevent any cyclic types from being declared across files.

Since that narrows our search to only cycles defined within a single file, we can skip walking to any other files in each check and allow the rule to catch them when the main visitor for the rules reaches them normally.

ex2
As an example, we can ignore the jump from file 1 to file 2 here, allow the visitor to traverse the types in the second file, and then catch the cycle starting from type-3 or type-4.

@dsinghvi dsinghvi force-pushed the main branch 11 times, most recently from 4eb9f86 to 9180bd7 Compare August 19, 2024 02:16
@Schnides123 Schnides123 changed the title Fix: Add validation rule for recursively-defined types (Issue #4135) fix: Add validation rule for recursively-defined types (Issue #4135) Aug 22, 2024
Adds logic and validations to detect cycles within type defintions.

code review changes
@Schnides123 Schnides123 changed the title fix: Add validation rule for recursively-defined types (Issue #4135) fix(cli): Add validation rule for recursively-defined types (Issue #4135) Aug 22, 2024
@Schnides123 Schnides123 requested a review from dsinghvi August 22, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants