drift_lint package

[dickermoshe]

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

[github-actions]

🚀 Deployed on https://deploy-preview-3224--moor.netlify.app

[simolus3]

How does custom_lint find plugins? I would prefer these to be part of drift_dev if possible instead of maintaining another package.

[dickermoshe]

Great point. But for some reason custom_lint only runs linters installed as dev_dependencies and some people install drift_dev into their main dependencies.

I've opened an issue for this invertase/dart_custom_lint#275

[dickermoshe]

@simolus3
This package is insanely cool
It took like 6 lines of code to do this:

[dickermoshe]

transaction lints: await all futures

[simolus3]

This is really cool! Something we should consider is whether we want to find a way to report all lints drift_dev is already reporting in builds via custom_lint as well.

As long as you have access to an analyzer context, it should be possible to create a drift analysis driver, run it on sources and then report issues found through there - I want to avoid duplicating logic for finding errors between lints and the build implementation. If this is not efficiently possible because the API exposed by custom_lints is too different, we should at least find a way to share as much as possible between the modes.

[dickermoshe]

This should be possible, that would be really cool!

[dickermoshe]

@simolus3 Is there a simple entry point where I can just pass in the AstNode of the DriftDatabase element and get a resolved driver?

I don't understand the generator code enough

[simolus3]

Essentially, this involves implementing DriftBackend - the most important part in there is readDart, which takes an URI and wants a resolved library. If you have an existing library from the resolved database element, you can implement that method by looking it up in a map (and, at the moment, throwing for other URIs). loadElementDeclaration may also be necessary, resolveUri can probably just call Uri.resolve and the other methods can throw for now.

Once you have a backend, you can use DriftAnalysisDriver(backend, const DriftOptions.defaults()) and driver.fullyAnalyze(inputUri) to run drift analysis. That returns a FileState, which has an allErrors getter returning lints with source spans.

This will get much trickier when we're dealing with file invalidation and re-analyzing files after edits, but I suspect just setting this all up is also pretty tough. The backend infrastructure is essentially meant as as a thin abstraction over the build packages and a test setup, so let me know if you get stuck or if there's anything I should look at.

[dickermoshe]

How cpu intensive is running all that?
If we run that on every edit, we might grind the analyzer to a halt

[simolus3]

Yeah that's what I'm worried about as well - I suspect it's not too bad because the analyzer itself is running again either way, and on builds that's what's making up most of the time.

Ideally we should have a way to do invalidation on a per-file basis, but it's not trivial because changes in imports trigger other files we have to analyze again and so on. Under the assumption that the analyzer is the most expensive thing, maybe running the entire drift analysis is fine at the beginning? If it turns out to be a problem, relatively cheap solutions are debouncing or introducing the existing cache we have for builds.

[dickermoshe]

@simolus3

I need you to implement this DriftBackend

1. Checkout this commit
2. Go to /docs
3. Run dart pub get
4. Run dart run custom_lint --watch
5. Go to drift_dev/lib/src/lints/drift_errors.dart
6. Implement the DriftBackend

[simolus3]

Done! It seems to work, but we need to find a way to emit errors with custom descriptions.

[dickermoshe]

Dude, this is insanely cool!

Opened a discussion here to see why we can't/shouldnt use dynamic errors

[dickermoshe]

Linting in the wrong spot:

[dickermoshe]

Ok, we got an answer

invertase/dart_custom_lint#277 (comment)

Users won't be able to disable individual lints.
We should add a codenme to each type of error. I'll work on that.

Thanks simon, I'll take it from here

[dickermoshe]

Linting in the wrong spot:

Duh, it's using the offset of the error in SQL without regard for the actual .dart file.

@simolus3 Any easy way to fix this?

[simolus3]

@simolus3 Any easy way to fix this?

If it's a single string literal, we can add the offset of the string in the Dart file to the SQL error. If it's not, we'll have to highlight the entire region as a precaution. I'll look into this, this also affects the existing warnings in builds.

Edit: Done in 7665813.

[dickermoshe]

Ok, we got an answer

invertase/dart_custom_lint#277 (comment)

Users won't be able to disable individual lints. We should add a codenme to each type of error. I'll work on that.

Thanks simon, I'll take it from here

After further review, it seems for disabling and enabling rules you need to create a single DartLintRule for each lint.
That would mean we would be running a build for each lint which is way too intense.

I'm just going to name this lint drift_build_error and users can either either disable or enable all of them.

[dickermoshe]

Great point. But for some reason custom_lint only runs linters installed as dev_dependencies and some people install drift_dev into their main dependencies. I've opened an issue for this invertase/dart_custom_lint#275

Fixed in invertase/dart_custom_lint#276