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 cljd.flutter.alpha2/widget clj-kondo support #194

Merged
merged 1 commit into from Feb 20, 2023

Conversation

D00mch
Copy link
Contributor

@D00mch D00mch commented Feb 12, 2023

Fix #191
Add directive support for:
:bg-watcher :let :managed :watch :vsync :context :get

Not fully supported:

  1. :bg-watcher is just ignored.
  2. Keys in a first place, like :dispose or :refresh-on in some binding forms are just ignored.

Not supported:
:flds destructuring. I will consider making a PR into clj-kondo, I have a commit in my local branch already

image

image

@borkdude
Copy link

borkdude commented Feb 13, 2023

@D00mch Thanks for this PR!

I'm willing to receive an issue about :flds but I'd have to think about how to better solve that since it's not valid in normal Clojure code, so I'll have to introduce special cases for ClojureDart.

@cgrand @dupuchba

About :flds: would it be possible to add features to ClojureDart without making syntax changes? This will in general work better with standard Clojure tooling.
E.g. in squint I've implemented field destructuring as ^:js {:keys [a b c]} (inspired by the js-interop library) and this requires no changes to clj-kondo or Cursive, etc.

@cgrand
Copy link
Contributor

cgrand commented Feb 13, 2023

@borkdude I see your point but it's not syntax change to us (for example we tried hard to not break the reader/cljc). Clojurescript introduced dashed properties, Clojure recently allowed to pass opts maps instead of spreading them etc.

The thing is that all these evolutions are non-breaking: they don't change the semantics of the rest of the language.
Language growth happens in un-specified spaces. If tooling treats unspecified as non-existent it stifles growth/innovation by creating inertia.

It reminds me of the open/closed world assumptions: Under the closed world assumption, if we don't know a thing, the thing doesn't exist and can't exist in the future without having to revise everything we inferred.
While in the open world assumption, what we know to exist, we know for sure and it won't go away but we can't reach conclusion about the non-existence of stuff we ignore. (This hints to methodic doubt in epistemology.)

(This echoes MUST UNDERSTAND/MUST IGNORE protocol design schools.)

My belief is that tooling should work under the open world assumption so as to not be in "crisis" when something new appears.

Concretely it would mean "warning on unknown" rather than "erroring on unknown", recognizing that some forms are correct, some are known to be incorrect (because they conflict with some knowledge) and some whose correctness is unknown and where the user's scrutiny is required.

I understand it's harder to create a good users experience in this setting but I deeply believe that the ecosystem as a whole would benefit from being more welcoming to accretive changes.

@borkdude
Copy link

@cgrand It's not so hard not to break the Clojure reader here, since most things are keywords, symbols, lists, etc. But it's very easy to change the language semantics (maybe I should have used that word) by introducing meaning to certain keywords. E.g. shadow-cljs introduced (:require ["foo.bar" :default baz]) to import a default value from the foo.bar library. But eventually this got replaced by "official" syntax in CLJS which now makes that custom syntax obsolete, but tools like clj-kondo and Cursive still end up having to maintain this. There is no general way in which a static analyzer can account for all kinds of new semantics without making bespoke changes for Clojure dialects. Clj-kondo doesn't croak or panic, it just doesn't know what :flds means at this point.

@borkdude
Copy link

borkdude commented Feb 13, 2023

I think the :flds issue was a slight digression from this PR: the PR can be merged independently from tools supporting :flds.

@borkdude
Copy link

I started a discussion about more broadly supporting :flds in the clojure-dev channel on Slack. Hope something will come out of it.

@cgrand
Copy link
Contributor

cgrand commented Feb 13, 2023

@borkdude

There is no general way in which a static analyzer can account for all kinds of new semantics without making bespoke changes for Clojure dialects.
Appart from running macros 😜

Well, to me divergence/convergence cycles are unavoidable. Clojure is thought as a hosted language and the host will always have to shine through.

It's not so hard not to break the Clojure reader here, since most things are keywords, symbols, lists, etc.

Well, when you have methods names such as [] or []=, or types like List<Widget Function(BuildContext context)>? or even methods named of<SomeClass> it's not always easy to shoehorn in the CLJC reader.

Metadata. ^ expects only a string, a symbol, a keyword or a map.
Strings for tags is an option but then it would mean parsing the string and dealing with ns aliases in the string (and tooling would have to learn to parse them).

@D00mch
Copy link
Contributor Author

D00mch commented Feb 16, 2023

So if anyone uses LSP, check it, please. The PR is ready, I can't do anything general with the :flds anyway

@dupuchba dupuchba merged commit 01ec972 into Tensegritics:main Feb 20, 2023
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.

False positive clj-kondo: unresolverd symbol in f/widget :watch
4 participants