[elpa] externals/javaimp f4216f2: Improve javaimp-organize-imports and friends

[Filipp Gunbin]

branch: externals/javaimp
commit f4216f241ed8b8d78a7ba234af00c7430563a294
Author: Filipp Gunbin <fgunbin@fastmail.fm>
Commit: Filipp Gunbin <fgunbin@fastmail.fm>

    Improve javaimp-organize-imports and friends
---
 javaimp-parse.el |  58 +++++++++++++++++--
 javaimp-tests.el |  32 ++++++++++-
 javaimp.el       | 166 +++++++++++++++++++++++--------------------------------
 3 files changed, 149 insertions(+), 107 deletions(-)

diff --git a/javaimp-parse.el b/javaimp-parse.el
index 3ff62c3..13950d2 100644
--- a/javaimp-parse.el
+++ b/javaimp-parse.el
@@ -31,6 +31,19 @@
 (defconst javaimp--parse-stmt-keyword-maxlen
   (seq-max (mapcar #'length javaimp--parse-stmt-keywords)))
 
+(defconst javaimp--parse-package-regexp
+  (javaimp--directive-regexp "package"))
+(defconst javaimp--parse-import-regexp
+  (javaimp--directive-regexp "import\\(?:[[:space:]]+static\\)?"))
+
+(defun javaimp--directive-regexp (directive)
+  "Return regexp suitable for matching package-like DIRECTIVE, a
+regexp.  First group is directive, second group is identifier."
+  (rx bol (* space)
+      (group (regexp directive)) (+ space)
+      (group (+ (any alnum ?_)) (* ?. (+ (any alnum ?_ ?*))))
+      (* space) ?\;))
+
 (defvar-local javaimp--parse-dirty-pos nil
   "Marker which points to a buffer position after which all parsed
 information should be considered as stale.  Usually set by
@@ -45,9 +58,9 @@ everything's up-to-date.")
       str)))
 
 (defun javaimp--parse-rsb-keyword (regexp &optional bound noerror count)
-  "Like `re-search-backward', but count only occurences outside
-syntactic context as given by `syntax-ppss-context'.  Assumes
-point is outside of any context initially."
+  "Like `re-search-backward', but count only occurences which start
+outside any syntactic context as given by `syntax-ppss-context'.
+Assumes point is outside of any context initially."
   (or count (setq count 1))
   (let ((step (if (>= count 0) 1 -1))
         (case-fold-search nil)
@@ -396,6 +409,7 @@ anywhere.  Makes it point nowhere when done."
 (defun javaimp--parse-setup-buffer ()
   ;; FIXME This may be done in major/minor mode setup
   (setq syntax-ppss-table javaimp-syntax-table)
+  (setq-local multibyte-syntax-as-symbol t)
   (add-hook 'after-change-functions #'javaimp--parse-update-dirty-pos))
 
 (defun javaimp--parse-class-abstract-methods ()
@@ -451,13 +465,34 @@ anywhere.  Makes it point nowhere when done."
 ;; responsibility.
 
 (defun javaimp--parse-get-package ()
-  "Return the package declared in the current file."
+  "Return the package declared in the current file.  Leaves point
+at the end of directive."
   (javaimp--parse-all-scopes)
   (goto-char (point-max))
-  (when (javaimp--parse-rsb-keyword
-         "^[ \t]*package[ \t]+\\([^ \t;\n]+\\)[ \t]*;" nil t 1)
+  (when (javaimp--parse-rsb-keyword javaimp--parse-package-regexp nil t 1)
+    (goto-char (match-end 0))
     (match-string 1)))
 
+(defun javaimp--parse-get-imports ()
+  "Parse import directives in the current buffer and return (REGION
+. CLASS-ALIST).  REGION, a cons of two positions, spans from bol
+of first import to eol of last import.  CLASS-ALIST contains
+elements (CLASS . TYPE), where CLASS is a string and TYPE is
+either of symbols `normal' or 'static'."
+  (javaimp--parse-all-scopes)
+  (goto-char (point-max))
+  (let (start-pos end-pos class-alist)
+    (while (javaimp--parse-rsb-keyword javaimp--parse-import-regexp nil t)
+      (setq start-pos (line-beginning-position))
+      (unless end-pos
+        (setq end-pos (line-end-position)))
+      (push (cons (match-string 2)
+                  (if (string-search "static" (match-string 1))
+                      'static 'normal))
+            class-alist))
+    (cons (and start-pos end-pos (cons start-pos end-pos))
+          class-alist)))
+
 (defun javaimp--parse-get-all-scopes (&optional pred parent-pred)
   "Return all scopes in the current buffer, optionally filtering
 them with PRED, and their parents with PARENT-PRED.  Neither of
@@ -488,4 +523,15 @@ them should move point."
     (seq-mapcat #'javaimp--parse-interface-abstract-methods
                 interfaces)))
 
+(defmacro javaimp--parse-without-hook (&rest body)
+  "Execute BODY, temporarily removing
+`javaimp--parse-update-dirty-pos' from `after-change-functions'
+hook."
+  (declare (debug t) (indent 0))
+  `(unwind-protect
+       (progn
+         (remove-hook 'after-change-functions 
#'javaimp--parse-update-dirty-pos)
+         ,@body)
+     (add-hook 'after-change-functions #'javaimp--parse-update-dirty-pos)))
+
 (provide 'javaimp-parse)
diff --git a/javaimp-tests.el b/javaimp-tests.el
index 16de66a..33b46a6 100644
--- a/javaimp-tests.el
+++ b/javaimp-tests.el
@@ -172,17 +172,43 @@ throws E1 {"
         (should (equal (javaimp-scope-name (car scopes)) (nth 2 item)))))))
 
 
-;; Tests for parsing
+;; Tests for parse "api"
 
 (ert-deftest javaimp-test--parse-get-package ()
   (with-temp-buffer
-    (insert "  package  foo.bar.baz  ;
+    (insert "
+  package  foo.bar.baz  ;
 //package commented.line;
 /*
 package commented.block;
-*/")
+*/
+")
     (should (equal (javaimp--parse-get-package) "foo.bar.baz"))))
 
+(ert-deftest javaimp-test--parse-get-imports ()
+  (with-temp-buffer
+    (insert "
+  import  some.class1  ;
+import  static  some.class.fun1;
+//import commented.line;
+/*
+import static commented.block;
+*/
+import someclass2;
+import my.package.* ;
+
+import static some_class.fun_2; // comment
+// comment outside
+")
+    (should (equal
+             (javaimp--parse-get-imports)
+             '((2 . 206)
+               ("some.class1" . normal)
+               ("some.class.fun1" . static)
+               ("someclass2" . normal)
+               ("my.package.*" . normal)
+               ("some_class.fun_2" . static))))))
+
 (ert-deftest javaimp-test--parse-get-all-scopes ()
   (with-temp-buffer
     (insert-file-contents
diff --git a/javaimp.el b/javaimp.el
index cbd1283..328a5ae 100644
--- a/javaimp.el
+++ b/javaimp.el
@@ -422,7 +422,7 @@ current module or source tree, see
                                eol))))))
      (list (completing-read "Import: " classes nil t nil nil
                             (symbol-name (symbol-at-point))))))
-  (javaimp-organize-imports (cons classname 'ordinary)))
+  (javaimp-organize-imports (list (cons classname 'normal))))
 
 (defun javaimp--get-jdk-classes (java-home)
   "If 'jmods' subdirectory exists in JAVA-HOME (Java 9+), read all
@@ -542,117 +542,87 @@ current buffer."
 ;; Organizing imports
 
 ;;;###autoload
-(defun javaimp-organize-imports (&rest new-imports)
-  "Groups import statements according to the value of
+(defun javaimp-organize-imports (&optional add-alist)
+  "Group import statements according to the value of
 `javaimp-import-group-alist' (which see) and prints resulting
-groups leaving one blank line between groups.
+groups putting one blank line between groups.
 
-If the file already contains some import statements, this command
-rewrites them, starting with the same place.  Else, if the the
-file contains package directive, this command inserts one blank
-line below and then imports.  Otherwise, imports are inserted at
-the beginning of buffer.
+If buffer already contains some import statements, put imports at
+that place.  Else, if there's a package directive, put imports
+below it, separated by one line.  Else, just put them at bob.
 
-Classes within a single group are ordered in a lexicographic
-order.  Imports not matched by any regexp in `javaimp-import-group-alist'
+Classes within a single group are sorted lexicographically.
+Imports not matched by any regexp in `javaimp-import-group-alist'
 are assigned a default order defined by
-`javaimp-import-default-order'.  Duplicate imports are squashed.
+`javaimp-import-default-order'.  Duplicate imports are elided.
 
-NEW-IMPORTS is a list of additional imports; each element should
-be of the form (CLASS . TYPE), where CLASS is a string and TYPE
-is `ordinary' or `static'.  Interactively, NEW-IMPORTS is nil."
+Additionally, merge imports from ADD-ALIST, an alist of the same
+form as CLASS-ALIST in return value of
+`javaimp--parse-get-imports'."
   (interactive)
   (barf-if-buffer-read-only)
   (save-excursion
-    (goto-char (point-min))
-    (let* ((old-data (javaimp--parse-imports))
-          (first (car old-data))
-          (last (cadr old-data))
-          (all-imports (append new-imports (cddr old-data))))
-      (if all-imports
-         (progn
-           ;; delete old imports, if any
-           (if first
-               (progn
-                 (goto-char last)
-                 (forward-line)
-                 (delete-region first (point))))
-           (javaimp--prepare-for-insertion first)
-           (setq all-imports
-                 (cl-delete-duplicates
-                   all-imports
-                   :test (lambda (first second)
-                           (equal (car first) (car second)))))
-           ;; assign order
-           (let ((with-order
+    (save-restriction
+      (widen)
+      (let ((parsed (javaimp--parse-get-imports)))
+        (when (or (cdr parsed) add-alist)
+          (javaimp--parse-without-hook
+            (javaimp--position-for-insert-imports (car parsed))
+            (let ((with-order
                   (mapcar
                    (lambda (import)
-                     (let ((order (or (assoc-default (car import)
-                                                     javaimp-import-group-alist
-                                                     'string-match)
-                                      javaimp-import-default-order)))
-                       (cons import order)))
-                   all-imports)))
+                     (let ((order
+                             (or (assoc-default (car import)
+                                                javaimp-import-group-alist
+                                               'string-match)
+                                javaimp-import-default-order)))
+                       (cons import order)))
+                    (delete-dups (append (cdr parsed) add-alist))))
+                  by-type)
              (setq with-order
                    (sort with-order
                          (lambda (first second)
-                           ;; sort by order, name
-                           (if (= (cdr first) (cdr second))
-                               (string< (caar first) (caar second))
-                             (< (cdr first) (cdr second))))))
-             (javaimp--insert-imports with-order)))
-        (message "Nothing to organize!")))))
-
-(defun javaimp--parse-imports ()
-  "Returns (FIRST LAST . IMPORTS)"
-  (let (first last imports)
-    (while (re-search-forward "^\\s *import\\s +\\(static\\s 
+\\)?\\([._[:word:]]+\\)" nil t)
-      (let ((type (if (match-string 1) 'static 'ordinary))
-           (class (match-string 2)))
-       (push (cons class type) imports))
-      (setq last (line-beginning-position))
-      (or first (setq first last)))
-    (cons first (cons last imports))))
-
-(defun javaimp--prepare-for-insertion (start)
-  (cond (start
-        ;; if there were any imports, we start inserting at the same place
-        (goto-char start))
-       ((re-search-forward "^\\s *package\\s " nil t)
-        ;; if there's a package directive, insert one blank line
-        ;; below it
-        (end-of-line)
-        (if (eobp)
-            (insert ?\n)
-          (forward-line))
-        (insert ?\n))
-       (t
-        ;; otherwise, just go to bob
-        (goto-char (point-min)))))
-
-(defun javaimp--insert-imports (imports)
-  (let ((static (seq-filter (lambda (elt)
-                             (eq (cdar elt) 'static))
-                           imports))
-       (ordinary (seq-filter (lambda (elt)
-                               (eq (cdar elt) 'ordinary))
-                             imports)))
-    (javaimp--insert-import-group "import static %s;" static)
-    (and static ordinary (insert ?\n))
-    (javaimp--insert-import-group "import %s;" ordinary)))
-
-(defun javaimp--insert-import-group (pattern imports)
-  (let (last-order)
+                           ;; sort by order then name
+                           (if (/= (cdr first) (cdr second))
+                                (< (cdr first) (cdr second))
+                             (string< (caar first) (caar second))))))
+              (setq by-type (seq-group-by #'cdar with-order))
+              (javaimp--insert-import-group
+               (cdr (assq 'normal by-type)) "import %s;\n")
+              (javaimp--insert-import-group
+               (cdr (assq 'static by-type)) "import static %s;\n"))
+            ;; Make sure there's only one blank line after
+            (forward-line -2)
+            (delete-blank-lines)
+            (end-of-line)
+            (insert ?\n)))))))
+
+(defun javaimp--position-for-insert-imports (old-region)
+  (if old-region
+      (progn
+        (delete-region (car old-region) (cdr old-region))
+        (goto-char (car old-region)))
+    (if (javaimp--parse-get-package)
+        (insert "\n\n")
+      ;; As a last resort, go to bob and skip comments
+      (goto-char (point-min))
+      (forward-comment (buffer-size))
+      (skip-chars-backward " \t\n")
+      (unless (bobp)
+        (insert "\n\n")))))
+
+(defun javaimp--insert-import-group (imports fmt)
+  (let (prev-order)
     (dolist (import imports)
-      ;; if adjacent imports have different order value, insert a newline
-      ;; between them
-      (let ((order (cdr import)))
-       (and last-order
-            (/= order last-order)
-            (insert ?\n))
-       (insert (format pattern (caar import)) ?\n)
-       (setq last-order order)))))
-
+      ;; If adjacent imports have different order value, insert a
+      ;; newline between them
+      (and prev-order
+          (/= (cdr import) prev-order)
+          (insert ?\n))
+      (insert (format fmt (caar import)))
+      (setq prev-order (cdr import)))
+    (when imports
+      (insert ?\n))))
 
 
 ;; Imenu support