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

failing to parse due to invalid symbol error #378

Closed
lread opened this issue Jul 27, 2019 · 8 comments
Closed

failing to parse due to invalid symbol error #378

lread opened this issue Jul 27, 2019 · 8 comments
Labels
bug Something isn't working
Projects

Comments

@lread
Copy link
Contributor

lread commented Jul 27, 2019

version
clj-kondo v2019.07.24-alpha

platform
macOS v10.14.6

problem
clj-kondo is failing with invalid symbol error

repro
Given the following src/borkdude.clj source:

(ns borkdude)

(defn -main []
  (println 'org/clojure/math.numeric-tower))

Sanity check:
clj -m borkdude outputs:

org/clojure/math.numeric-tower

expected behavior
clj-kondo --lint src outputs no errors.

actual behavior
clj-kondo --lint src outputs:

src/borkdude.clj:0:0: error: can't parse src/borkdude.clj, Invalid symbol: org/clojure/math.numeric-tower.
linting took 3ms, errors: 1, warnings: 0
@borkdude borkdude added this to the release 23 milestone Jul 28, 2019
@borkdude borkdude added the bug Something isn't working label Jul 28, 2019
@borkdude borkdude modified the milestones: release 23, release 24 Jul 31, 2019
@yuhan0
Copy link
Contributor

yuhan0 commented Aug 7, 2019

'org/clojure/math.numeric-tower is actually an illegal symbol according to the language spec:
https://clojure.org/reference/reader#_symbols

I think the current Clojure reader just happens not to enforce that rule, whereas clj-kondo's parser is stricter. And it may change in the future, see https://clojure.atlassian.net/browse/CLJ-1530

The expected behaviour should be to output a lint error pointing to the actual location of the symbol, i.e.

src/borkdude.clj:4:11: error: Invalid symbol: org/clojure/math.numeric-tower.

Here are some other read-time errors that cause the linter to fail for the entire file and output a "can't parse" error at 0:0

clojure.core/ ; <= invalid symbol
:             ; <= invalid keyword
10z           ; <= invalid number
\u12345       ; <= invalid unicode literal

I might try tackling this in the next day or two :)

@borkdude
Copy link
Member

borkdude commented Aug 7, 2019

@yuhan0 Actually I made a similar patch for keywords to let clj-kondo be less strict about it, since Clojure also accepts it right now and someone was using this to represent CSS using the garden library.

See: https://github.com/borkdude/clj-kondo/blob/b9d52ff9e0109bb55e4c490beb767ddebbf25870/parser/clj_kondo/impl/rewrite_clj/parser/keyword.clj#L1

The issue: #301

I think the same behavior is desirable for symbols to be consistent. Once Clojure itself enforces it, we can also enforce it in this linter.

Regardless, the location of the error can be improved.

@borkdude borkdude modified the milestones: release 24, release 25 Aug 9, 2019
@borkdude borkdude modified the milestones: release 25, release 26 Sep 22, 2019
@borkdude borkdude removed this from the release 26 milestone Oct 3, 2019
@borkdude borkdude added this to Low priority in clj-kondo Oct 15, 2019
@turtlegrammar
Copy link

I'm getting Can't parse <stdin>, Key must be integer on the following file:

(ns name)

(def x (String. "hello"))

Removing the String. resolves the problem -- it looks like it's the . at the end of the symbol causing the problem. This strikes me as more serious than the other examples in this thread, because I think it's valid per the language spec.

I'm using the v2020.7.29 VSCode extension.

@borkdude
Copy link
Member

borkdude commented Aug 7, 2020

@disalvjn Please try to lint this file without any of your project config. Your project config is likely the problem. I can't reproduce it.

@turtlegrammar
Copy link

@borkdude You're right; the issue was with my project config. I had:

{:linters 
  {:unresolved-symbol [(faconne.core/transform) (cats.core/mlet)]}}

when I needed to wrap the vector in :exclude like

{:linters 
 {:unresolved-symbol 
    {:exclude [(faconne.core/transform) (cats.core/mlet)]}}}

Thanks for your speedy response and your help!

@borkdude
Copy link
Member

@lread Can we close this issue or do you think clj-kondo should handle this differently?

@lread
Copy link
Contributor Author

lread commented Oct 19, 2020

Yeah, I think we can close, thanks for asking.

FWIW, I've decided not to support this illegal syntax, at least for now, in rewrite-cljc.

@lread lread closed this as completed Oct 19, 2020
clj-kondo automation moved this from Low priority to Done Oct 19, 2020
@sogaiu
Copy link
Contributor

sogaiu commented Oct 19, 2020

FYI: carocad/parcera#94 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
clj-kondo
  
Done
Development

No branches or pull requests

5 participants