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

Linter for drift #3281

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Linter for drift #3281

wants to merge 9 commits into from

Conversation

dickermoshe
Copy link
Collaborator

@dickermoshe dickermoshe commented Oct 11, 2024

Dunno what happened to the old PR. I think the VScode Github Plugin must've closed it. Anyway, here's it as a new PR

  • createReturning with ignore mode
  • offset without limit
  • await all migrations
  • Manager report issues

@dickermoshe
Copy link
Collaborator Author

dickermoshe commented Oct 11, 2024

@simolus3

See how I setup the tests. Lmk if you have a better way of setting them up.

Also, Some of the async code is copied from the dart sdk, I don't know how this affects licensing.

@dickermoshe dickermoshe self-assigned this Oct 11, 2024
@dickermoshe dickermoshe marked this pull request as draft October 11, 2024 04:00
Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Some comments on the test infrastructure

@dickermoshe
Copy link
Collaborator Author

dickermoshe commented Oct 14, 2024

Right now warnings in manager generation are not reported in the resolver, so the linter can't use them

The warning I'm focusing on are duplicate reverse names e.g.

class User extends Table {
  late final group = integer().references(Group, #id)
  late final supergroup = integer().references(Group, #id)
}
class Group extends Table {
//   `userRefs` has 2 relations!!!
}

How would I go about this?

@dickermoshe
Copy link
Collaborator Author

I add a warning level to DriftAnalysisError to show orange squiggles instead of red for less severe errors

@simolus3
Copy link
Owner

Right now warnings in manager generation are not reported in the resolver, so the linter can't use them

Ideally, these warnings should be discovered in the analysis step instead of during code generation. I know that some of them are easier to report during codegen, is it a lot of work to port them over to something that could run in FileAnalyzer? The parts under if (element is BaseDriftAccessor) should have the same information available to them as the generator, especially in the end when all transitive tables have been added). I can also get started on this to set up the basic infrastructure for those warnings if that works for you.

@dickermoshe
Copy link
Collaborator Author

Yeah, could you please do that?
I tried myself, but after an hour I gave up.

@dickermoshe
Copy link
Collaborator Author

dickermoshe commented Oct 27, 2024

@simolus3 Any update on this.
Just need you to write some scaffolding where to do these analysis.

@dickermoshe
Copy link
Collaborator Author

In FileAnalyzer I don't have access to the base element to report the error

@dickermoshe
Copy link
Collaborator Author

Ping

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