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

f-s must be run binding *read-eval* to false #158

Open
vemv opened this issue Apr 25, 2020 · 1 comment
Open

f-s must be run binding *read-eval* to false #158

vemv opened this issue Apr 25, 2020 · 1 comment

Comments

@vemv
Copy link
Contributor

vemv commented Apr 25, 2020

Problem statement

Some .clj files can contain the reader-eval syntax. https://clojure.org/guides/weird_characters#_reader_eval says:

Note that #= is not an officially supported feature of the reader, so you shouldn’t rely on its presence in future versions of Clojure.

People can include this #= syntax in their .clj files, for reasons (personally I don't recommend so).

When tools.reader, used by our strategies (and potentially used by any of the libs we wrap) encounters a #=(), it will actually trigger Clojure evaluation, while *ns* is bound to a different ns to the one being read.

So if foo.clj contains #=(release-sharks), whenever some other ns attempts to read this file, tools.reader will invoke release-sharks, attempting to resolve it to *ns* (not foo).

This is wrong at several levels:

  • Running code is undesired to begin with
    • Can create very surprising behavior - e.g. running f-s should not cause application-level side-effects
  • because *ns* is bound to a different ns to the one being read, it's very unlikely that function invocations will succeed
    • This causes an exception, making the whole file unreadable.

Proposal

Bind *read-eval* to false in the right place, such that it's always false when running any strategy, linter, formatter, etc.

AC

Ideally, a .clj file containing #=(foo) can be formatted and linted.

Currently, such a file would be excluded at a strategies level:

Alternatives and comparison

  • Bind *ns* to the ns being read
    • This complects code loading/running (a concern of e.g. tools.namespace) with formatting/linting. Generally f-s should not trigger application-level side-effects, perform excessive requires, etc.
@vemv
Copy link
Contributor Author

vemv commented Apr 25, 2020

⚠️to be done after #152

vemv added a commit to reducecombine/formatting-stack that referenced this issue Mar 17, 2021
This particularly targets Eastwood (since it's the only member of our stack that evals code, and therefore can run read-eval syntax)
and tools.reader (which we internally use for various purposes).

It also is bound at `formatting-stack.core` level in case any other mechanisms happen to be sensitive.

Fixes nedap#158
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 a pull request may close this issue.

1 participant