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

add symbol validation API #128

Merged
merged 1 commit into from
Nov 3, 2020
Merged

add symbol validation API #128

merged 1 commit into from
Nov 3, 2020

Conversation

neetopia
Copy link
Contributor

No description provided.

@neetopia neetopia requested a review from ting-yuan October 23, 2020 22:22
Copy link
Collaborator

@ting-yuan ting-yuan left a comment

Choose a reason for hiding this comment

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

Have you tried on a larger project to see if the visitor is complete?

@@ -78,6 +79,14 @@ fun KSDeclaration.isLocal(): Boolean {
return this.parentDeclaration != null && this.parentDeclaration !is KSClassDeclaration
}

/**
* Perform a validation on a given symbol to check if all interested types in symbols enclosed scope are valid.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you define valid? E.g., all immediate type references are resolvable.

@@ -78,6 +79,14 @@ fun KSDeclaration.isLocal(): Boolean {
return this.parentDeclaration != null && this.parentDeclaration !is KSClassDeclaration
}

/**
* Perform a validation on a given symbol to check if all interested types in symbols enclosed scope are valid.
* @param shouldValidate: A lambda for filtering interested symbols. Default checks all.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe explain why filtering is needed, i.e., performance.

}

override fun visitPropertyDeclaration(property: KSPropertyDeclaration, data: Unit): Boolean {
if (property.type.resolve().isError) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be property.type.accept(this, Unit), so that shouldValidate has a chance to skip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea here is to skip before jumping into this check, since when checking a property, its type should be valid for the property to be valid.

* Perform a validation on a given symbol to check if all interested types in symbols enclosed scope are valid, i.e. resolvable.
* @param predict: A lambda for filtering interested symbols for performance purpose. Default checks all.
*/
fun KSNode.validate(predict: (KSNode, KSNode) -> Boolean = { _, _-> true } ): Boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/predict/predicate/g

@neetopia neetopia merged commit 3bcf10c into google:master Nov 3, 2020
@neetopia neetopia deleted the validate branch November 3, 2020 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants