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

Improve Type Assertion Handling in Compiler.Compile #1894

Open
knbr13 opened this issue Jan 9, 2025 · 0 comments
Open

Improve Type Assertion Handling in Compiler.Compile #1894

knbr13 opened this issue Jan 9, 2025 · 0 comments

Comments

@knbr13
Copy link

knbr13 commented Jan 9, 2025

Hi Permify team,

I recently discovered your project, and it looks very promising!
I'm interested in contributing and have started exploring the code.

While reviewing the Compile function in pkg/dsl/compiler/compiler.go, I noticed an opportunity for improvement regarding type assertions.

In this section: link to code at line 49

Currently, the type assertion result is used multiple times inside the switch cases. To make the code cleaner and more efficient, we could assign the result of the type assertion to a variable once and then use that variable inside the switch. This would eliminate the need for multiple type assertions.

The staticcheck tool also reports this issue:

pkg/dsl/compiler/compiler.go:49:10: assigning the result of this type assertion to a variable (switch statement := statement.(type)) could eliminate type assertions in switch cases (S1034)
pkg/dsl/compiler/compiler.go:52:27: could eliminate this type assertion
pkg/dsl/compiler/compiler.go:68:25: could eliminate this type assertion

If you agree with this suggestion, I'd be happy to take care of the fix. Please feel free to assign the issue to me!

Thanks!

Note: even if we keep the assertions in the switch cases, we don't need the ok check (v, ok := s.(type)). We can simplify this to just v := s.(type) since the assertion is already performed. So for example, this check will never evaluate to true, since ok is always true: link to code at line 53. But anyway, assigning the result of the type assertion to a variable will eliminate the need for both, we will no longer need to do type assertions again in switch cases.

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

No branches or pull requests

1 participant