Skip to content

analyzer: introduce a :compile-like key #14

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

Merged
merged 2 commits into from
Sep 29, 2023
Merged

analyzer: introduce a :compile-like key #14

merged 2 commits into from
Sep 29, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Sep 29, 2023

Context

In CIDER, when I type (.foo "") at the repl, no stacktrace should be displayed, because it adds nothing to the message that already will be displayed.

However, this is a runtime exception, not a compile-one, so we can't use our new :phase logic.

I asked about that here: https://ask.clojure.org/index.php/13339/tools-better-detect-errors-coming-existing-methods-fields

Changes

Introduce a :compile-like key indicating if we deem the exception "compile-like".

We can expand the meaning of it as we find more, similar cases.

Cheers - V

m {:class (name (:type cause-data))
:phase (-> cause-data :data :clojure.error/phase)
:phase phase
Copy link
Member Author

Choose a reason for hiding this comment

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

At first I was tempted to put a synthetic value under :phase, but that might be surprising to anyone using Haystack as a lib.

I plan to do some transparent changes in cider.el such that we can we can change this default:

(defcustom cider-clojure-compilation-error-phases '("read-source"
                                                    "macro-syntax-check"
                                                    "macroexpansion"
                                                    "compile-syntax-check"
                                                    "compilation"
+                                                   "cider/compile-like"

@vemv vemv requested a review from bbatsov September 29, 2023 04:30
@@ -338,14 +338,25 @@
(flag-duplicates)
(flag-tooling)))))

(defn- compile-like-exception?
Copy link
Member

Choose a reason for hiding this comment

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

I think you should add some of your explanations from the PR here, so it's clear why we need this.

@bbatsov
Copy link
Member

bbatsov commented Sep 29, 2023

Introduce a :compile-like key indicating if we deem the exception "compile-like".

Not sure I like the term, but I can't propose a better one and the PR looks good overall.

CHANGELOG.md Outdated
@@ -2,6 +2,10 @@

## master (unreleased)

* `analyzer`: include a `:compile-like` key which indicates if the error happened at a "compile-like" phase.
* It represents exceptions that happen at runtime (and therefore never include a `:phase`) which however, represent code that cannot possibly work, and therefore are a 'compile-like' exception (i.e. a linter could have caught them).
Copy link
Member

Choose a reason for hiding this comment

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

You used the double quotes above and single here and below. This triggers my OCD. :D

@vemv
Copy link
Member Author

vemv commented Sep 29, 2023

Amended with feedback - thanks!

@vemv vemv merged commit 080c92e into master Sep 29, 2023
@vemv vemv deleted the compile-like branch September 29, 2023 11:57
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