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

improve emacs support #353

Merged
merged 7 commits into from
Jun 13, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
plugin/emacs: add Eglot support
This commit also ignores client notifications that would make
quicklintjs to crash and accept "js" as a language_id for files,
that's what Eglot uses for JavaScript.

Signed-off-by: wagner riffel <[email protected]>
wgrr committed Jun 13, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 54bc9ebd31fac7170fca963230ab30e427c610a3
1 change: 1 addition & 0 deletions plugin/emacs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ include(GNUInstallDirs)

install(
FILES flycheck-quicklintjs.el
FILES eglot-quicklintjs.el
DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/emacs/site-lisp"
)

62 changes: 62 additions & 0 deletions plugin/emacs/eglot-quicklintjs.el
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
;;; eglot-quicklintjs.el --- Eglot support for quick-lint-js -*- lexical-binding: t; -*-

;;; Commentary:

;; Eglot support for quick-lint-js.

;; Example usage in your init.el:
;;
;; (require 'eglot-quicklintjs)
;;
;; (defun my-eglot-quicklintjs-setup ()
;; "Configure eglot-quicklintjs for better experience."
;;
;; ;; Remove the time to wait after last change before automatically checking
;; ;; buffer. The default is 0.5 (500ms)
;; (setq-local eglot-send-changes-idle-time 0))
;; (add-hook 'js-mode-hook #'my-eglot-quicklintjs-setup)

;;; Code:

(require 'eglot)

(defgroup eglot-quicklintjs nil
"quick-lint-js Eglot integration."
:group 'eglot-quicklintjs
:link '(url-link :tag "Website" "https://quick-lint-js.com"))

(defcustom eglot-quicklintjs-program "quick-lint-js"
"Path to quick-lint-js program to run."
:group 'eglot-quicklintjs
:type 'stringp)

(defcustom eglot-quicklintjs-args nil
"Arguments to quick-lint-js."
:group 'eglot-quicklintjs
:type '(repeat string))

(add-to-list 'eglot-server-programs `(js-mode . (,eglot-quicklintjs-program
"--lsp-server"
,@eglot-quicklintjs-args)))

(provide 'eglot-quicklintjs)

;; quick-lint-js finds bugs in JavaScript programs.
;; Copyright (C) 2020 Matthew Glazar
;;
;; This file is part of quick-lint-js.
;;
;; quick-lint-js is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.
;;
;; quick-lint-js is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
;; GNU General Public License for more details.
;;
;; You should have received a copy of the GNU General Public License
;; along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.

;;; eglot-quicklintjs.el ends here
32 changes: 21 additions & 11 deletions plugin/emacs/test/quicklintjs-test.el
Original file line number Diff line number Diff line change
@@ -9,6 +9,12 @@
(expand-file-name default-directory)
".melpa-cache/"))

(defun quicklintjs-install-deps (deps)
(mapcar (lambda (pkg) (unless (package-installed-p pkg)
(if (> emacs-major-version 24)
(package-install pkg t)
(package-install pkg)))) deps))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Indentation looks off to me. Shouldn't there be two more spaces before this line to align it with line 15?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been indenting with a script that runs emacs commad "indent-region" in the file, it does indent as you're looking, and lisp code inside emacs /lisp directory is also indented like this:

 (if (> 0 0) ; cond
     (foo) ; then
   (bar)   ; else

so i think it's correct according to Emacs standards


(defun quicklintjs-test-main ()
(setq package-user-dir cache-dir-name
package-check-signature nil)
@@ -19,23 +25,27 @@
(unless package-archive-contents
(package-refresh-contents))

(unless (package-installed-p 'flycheck)
;; the DONT-SELECT argument is only available and make sense
;; in emacs 25 and above.
(if (> emacs-major-version 24)
(package-install 'flycheck t)
(package-install 'flycheck)))
(quicklintjs-install-deps (if (>= emacs-major-version 26)
'(flycheck eglot)
'(flycheck)))
(def-flycheck-tests)
(def-eglot-tests)
(ert-run-tests-batch-and-exit))

(defun def-eglot-tests ()
(when (>= emacs-major-version 26)
(require 'eglot-quicklintjs)
(ert-deftest quicklintjs-is-in-eglot-servers ()
(should (member '(js-mode "quick-lint-js" "--lsp") eglot-server-programs)))))

(defun def-flycheck-tests ()
(require 'flycheck)
(require 'flycheck-ert)
(require 'flycheck-quicklintjs)
(def-flycheck-tests)
(ert-run-tests-batch-and-exit))

(ert-deftest quicklintjs-is-in-checkers ()
(should (member 'javascript-quicklintjs flycheck-checkers)))
(ert-deftest quicklintjs-is-in-flycheck-checkers ()
(should (member 'javascript-quicklintjs flycheck-checkers)))

(defun def-flycheck-tests ()
(flycheck-ert-def-checker-test
javascript-quicklintjs javascript error
(let ((flycheck-checker 'javascript-quicklintjs)
8 changes: 7 additions & 1 deletion src/lsp-server.cpp
Original file line number Diff line number Diff line change
@@ -120,6 +120,12 @@ void linting_lsp_server_handler<Linter>::handle_notification(
std::exit(this->shutdown_requested_ ? 0 : 1);
} else if (starts_with(method, "$/"sv)) {
// Do nothing.
} else if (method == "workspace/didChangeConfiguration") {
// Do nothing.
} else if (method == "textDocument/didSave") {
// Do nothing.
} else if (method == "textDocument/willSave") {
// Do nothing.
} else {
QLJS_UNIMPLEMENTED();
}
@@ -266,7 +272,7 @@ void linting_lsp_server_handler<Linter>::
doc.doc.set_text(make_string_view(text_document["text"]));
doc.version_json = get_raw_json(version);

if (language_id == "javascript") {
if (language_id == "javascript" || language_id == "js") {
doc.type = document_type::lintable;
this->linter_.lint_and_get_diagnostics_notification(
*this->get_config(document_path), doc.doc.string(), get_raw_json(uri),
63 changes: 63 additions & 0 deletions test/test-lsp-server.cpp
Original file line number Diff line number Diff line change
@@ -284,6 +284,69 @@ TEST_F(test_linting_lsp_server, opening_document_lints) {
EXPECT_THAT(this->lint_calls, ElementsAre(u8"let x = x;"));
}

TEST_F(test_linting_lsp_server, opening_document_language_id_js_lints) {
this->lint_callback = [&](configuration&, padded_string_view code,
string8_view uri_json, string8_view version_json,
byte_buffer& notification_json) {
EXPECT_EQ(code, u8"let x = x;");
EXPECT_EQ(uri_json, u8"\"file:///test.js\"");
EXPECT_EQ(version_json, u8"10");
notification_json.append_copy(
u8R"--(
{
"method":"textDocument/publishDiagnostics",
"params":{
"uri": "file:///test.js",
"version": 10,
"diagnostics": [
{
"range": {
"start": {"line": 0, "character": 8},
"end": {"line": 0, "character": 9}
},
"severity": 1,
"message": "variable used before declaration: x"
}
]
},
"jsonrpc":"2.0"
}
)--");
};

this->server.append(
make_message(u8R"({
"jsonrpc": "2.0",
"method": "textDocument/didOpen",
"params": {
"textDocument": {
"uri": "file:///test.js",
"languageId": "js",
"version": 10,
"text": "let x = x;"
}
}
})"));

ASSERT_EQ(this->client.messages.size(), 1);
::Json::Value& response = this->client.messages[0];
EXPECT_EQ(response["method"], "textDocument/publishDiagnostics");
EXPECT_FALSE(response.isMember("error"));
// LSP PublishDiagnosticsParams:
EXPECT_EQ(response["params"]["uri"], "file:///test.js");
EXPECT_EQ(response["params"]["version"], 10);
::Json::Value& diagnostics = response["params"]["diagnostics"];
EXPECT_EQ(diagnostics.size(), 1);
EXPECT_EQ(diagnostics[0]["range"]["start"]["line"], 0);
EXPECT_EQ(diagnostics[0]["range"]["start"]["character"], 8);
EXPECT_EQ(diagnostics[0]["range"]["end"]["line"], 0);
EXPECT_EQ(diagnostics[0]["range"]["end"]["character"], 9);
EXPECT_EQ(diagnostics[0]["severity"], lsp_error_severity);
EXPECT_EQ(diagnostics[0]["message"], "variable used before declaration: x");

EXPECT_THAT(this->lint_calls, ElementsAre(u8"let x = x;"));
}

TEST_F(test_linting_lsp_server, changing_document_with_full_text_lints) {
this->server.append(
make_message(u8R"({