-
Notifications
You must be signed in to change notification settings - Fork 102
chore: expose schema_cache & file_context in lint rules #449
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason why we do not simply put these into the rule context? from an API standpoint, I feel like they should?
ctx: &RuleContext<Self>, | ||
_file_context: &AnalysedFileContext, | ||
_schema_cache: Option<&SchemaCache>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from an API standpoint, I would put these two into the RuleContext
. After all, they are "context" info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt the RuleContext
held static metadata about the rule, while the other stuff is related to the db/data model and the parsed sql file.
But I don't have a hard opinion
@@ -0,0 +1,7 @@ | |||
#[derive(Default)] | |||
pub struct AnalysedFileContext {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just a scaffold for the file context.
We can add properties in future linting PRs.
eugene works the same way: link
Analyser::run
now takesVec<AnalysableStatement>
and anOption<&SchemaCache>
AnalysedFileContext
SchemaCache
and theAnalysedFileContext
are passed into each linting rule.This should roughly match how the eugene code base does it.
The rest of the changes are adaptions to those changes or minor things, e.g. a new document-mapper or some renamings.