[elpa] externals/javaimp 8f285d7: Add javaimp-tests-maven.el, cosmetic changes in javaimp-maven.el

[Filipp Gunbin]

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

    Add javaimp-tests-maven.el, cosmetic changes in javaimp-maven.el
---
 javaimp-maven.el               |  63 ++++++++++++++------------------
 javaimp-tests-maven.el         |  80 +++++++++++++++++++++++++++++++++++++++++
 testdata/maven-projects.tar.gz | Bin 0 -> 1217 bytes
 3 files changed, 107 insertions(+), 36 deletions(-)

diff --git a/javaimp-maven.el b/javaimp-maven.el
index 73d914d..48b3a1b 100644
--- a/javaimp-maven.el
+++ b/javaimp-maven.el
@@ -41,22 +41,12 @@ resulting module trees."
         (modules (mapcar (lambda (proj-elt)
                             (javaimp--maven-module-from-xml proj-elt file))
                           projects)))
-
-    ;; Set :file slots in a separate step because Maven doesn't seem
-    ;; to give pom.xml file location for each subproject.  We build
-    ;; tree below by using child->parent links (<parent> in child
-    ;; pom.xml), that also doesn't help to find pom.xml itself.
-    ;; Furthermore, the reverse link (<modules> in parent) may not
-    ;; correspond to a project's real children.  So we take this
-    ;; approach: starting from the top-level pom.xml, descend into
-    ;; children using <modules>.  For each child pom.xml, find its
-    ;; corresponding module by id in the whole module list, and set
-    ;; that module's :file.
+    ;; Set :file slots in a separate step because Maven doesn't give
+    ;; pom.xml file location for each subproject.
     (javaimp--maven-fill-modules-files file modules)
-    (when-let ((without-files
-                (seq-filter (lambda (m)
-                              (null (javaimp-module-file m)))
-                            modules)))
+    (when-let ((without-files (seq-filter (lambda (m)
+                                            (null (javaimp-module-file m)))
+                                          modules)))
       (error "Cannot find file for module(s): %s"
              (mapconcat (lambda (m)
                           (javaimp-print-id (javaimp-module-id m)))
@@ -64,15 +54,15 @@ resulting module trees."
                         ", ")))
     (dolist (m modules)
       (setf (javaimp-module-raw m) nil)) ;no longer need it
-
+    ;; Build project trees
     (let ((roots
            (cons
             (car modules)               ;first module is always root
-            ;; In some rare cases, a project inside a tree has
-            ;; parent outside the tree (examples: "log4j-bom" in
+            ;; In some rare cases, a project inside a tree has parent
+            ;; outside the tree (examples: "log4j-bom" in
             ;; log4j-2.11.2, "external" in jaxb-ri-2.3.3).  Such
-            ;; projects didn't get it into the tree, and we need
-            ;; to add them a separate roots.
+            ;; projects didn't get into the tree, and we need to add
+            ;; them a separate roots.
             (seq-filter (lambda (m)
                           (not (seq-find (lambda (aux)
                                            (equal (javaimp-module-id aux)
@@ -84,9 +74,8 @@ resulting module trees."
                          (javaimp-print-id (javaimp-module-id root)))
                 (javaimp--build-tree
                  root modules
-                ;; more or less reliable way to find
-                ;; children is to look for modules with
-                ;; "this" as the parent
+                ;; more or less reliable way to find children is to
+                ;; look for modules with "this" as the parent
                  (lambda (el tested)
                    (equal (javaimp-module-parent-id tested)
                           (javaimp-module-id el)))))
@@ -104,7 +93,7 @@ resulting module trees."
           (progn
             (goto-char (point-min))
             (re-search-forward "<\\(projects?\\)")
-            ;; corresponding close tag is the end of parse region
+            ;; find corresponding close tag
             (search-forward (concat "</" (match-string 1) ">"))
             (match-end 0)))))
     (xml-parse-region start end)))
@@ -149,10 +138,12 @@ resulting module trees."
    :version (javaimp--xml-first-child (javaimp--xml-child 'version elt))))
 
 (defun javaimp--maven-fill-modules-files (file modules)
-  ;; Reads module id from FILE, looks up corresponding module in
-  ;; MODULES, sets its :file slot, then recurses for each submodule.
-  ;; A submodule file path is constructed by appending relative path
-  ;; taken from <module> to FILE's directory.
+  "Reads <module> ids from FILE, looks up corresponding modules in
+MODULES, sets their :file slot, then recurses for each of them.
+A submodule file path is constructed by appending relative path
+taken from <module> to FILE's directory.  This seems to be the
+most reliable way, because relationships between modules in Maven
+are somewhat arbitrary."
   (let* ((xml-tree (with-temp-buffer
                     (insert-file-contents file)
                     (xml-parse-region (point-min) (point-max))))
@@ -181,14 +172,14 @@ resulting module trees."
 
     (setf (javaimp-module-file module) file)
 
-    ;; Take <module> subelements from raw project, not from file,
-    ;; because Maven can extend the list with elements from profiles.
-    (let ((rel-paths
-           (mapcar #'javaimp--xml-first-child
-                  (javaimp--xml-children
-                    (javaimp--xml-child 'modules (javaimp-module-raw module))
-                    'module))))
-      ;; recurse into each child <module>
+    ;; Take <module> subelements from information we've got from
+    ;; help:effective-pom because Maven can extend the list in the
+    ;; file with elements from profiles
+    (let* ((submodules-elt
+            (javaimp--xml-child 'modules (javaimp-module-raw module)))
+           (submodules (javaimp--xml-children submodules-elt 'module))
+           (rel-paths (mapcar #'javaimp--xml-first-child submodules)))
+      ;; recurse into each submodule
       (dolist (rel-path rel-paths)
        (javaimp--maven-fill-modules-files
          (concat (file-name-directory file)
diff --git a/javaimp-tests-maven.el b/javaimp-tests-maven.el
new file mode 100644
index 0000000..49fbb66
--- /dev/null
+++ b/javaimp-tests-maven.el
@@ -0,0 +1,80 @@
+;;; javaimp-tests-maven.el --- javaimp Maven tests  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2021-2021  Free Software Foundation, Inc.
+
+;; Author: Filipp Gunbin <fgunbin@fastmail.fm>
+;; Maintainer: Filipp Gunbin <fgunbin@fastmail.fm>
+
+(require 'ert)
+(require 'javaimp)
+
+;; Tests for Maven project parsing.
+
+;; In the untarred directory, use this command to run Maven manually,
+;; if needed for debugging:
+;;
+;; `mvn -U -s settings.xml -f single/pom.xml help:effective-pom'.
+;;
+;; Dummy settings.xml file:
+;;
+;; `<?xml version="1.0" encoding="UTF-8"?><settings/>'
+;;
+
+(ert-deftest javaimp-test--maven-visit ()
+  (let* ((tmpdir (file-name-as-directory (make-temp-file "javaimp" t)))
+         (rc (call-process "tar" nil nil nil
+                           "-x"
+                           "-f" (concat javaimp--basedir
+                                        "testdata/maven-projects.tar.gz")
+                           "-C" tmpdir)))
+    (unless (= rc 0)
+      (error "Cannot untar test data: %d" rc))
+    ;;
+    ;; Single-module project
+    (should
+     (equal
+      (javaimp-test--maven-get-tree
+       (concat tmpdir (file-name-as-directory "single")))
+      '((("single:org.example:1.0.0" . "pom.xml")))))
+    ;;
+    ;; Multi-module project
+    (should
+     (equal
+      (javaimp-test--maven-get-tree
+       (concat tmpdir (file-name-as-directory "multi")))
+      '(;; Main tree:
+        (("multi:org.example:1.0.0" . "pom.xml")
+         (("child:org.example:1.0.0" . "child/pom.xml"))
+         ;; inherited group/version
+         (("child-only-artifact:org.example:1.0.0" . 
"child-only-artifact/pom.xml"))
+         ;; aggregator1 has no parent, but its child has
+         (("aggregator1-child-of-parent:org.example:1.0.0" .
+           "aggregator1/aggregator1-child-of-parent/pom.xml"))
+         (("aggregator2:org.example:1.0.0" . "aggregator2/pom.xml")
+          (("aggregator2-child-of-aggregator2:org.example:1.0.0" .
+            "aggregator2/aggregator2-child-of-aggregator2/pom.xml")))
+         ;; contained in aggregator2, but parent is above
+         (("aggregator2-child-of-parent:org.example:1.0.0" .
+           "aggregator2/aggregator2-child-of-parent/pom.xml")))
+        ;;
+        ;; Some projects end up outside of main tree:
+        ;; - because no parent
+        (("submodule-no-parent:org.example:1.0.0" . 
"submodule-no-parent/pom.xml"))
+        ;; - because parent is outside
+        (("with-parent-outside:org.example:1.0.0" . 
"with-parent-outside/pom.xml"))
+        ;; - because no parent
+        (("aggregator1:org.example:1.0.0" . "aggregator1/pom.xml")
+         (("aggregator1-child-of-aggregator1:org.example:1.0.0" .
+           "aggregator1/aggregator1-child-of-aggregator1/pom.xml")))
+        )))))
+
+(defun javaimp-test--maven-get-tree (project-dir)
+  (javaimp--map-nodes
+   (lambda (mod)
+     (cons t
+           (cons (javaimp-print-id (javaimp-module-id mod))
+                 (file-relative-name (javaimp-module-file mod) project-dir))))
+   #'always
+   (javaimp--maven-visit (concat project-dir "pom.xml"))))
+
+(provide 'javaimp-tests-maven)
diff --git a/testdata/maven-projects.tar.gz b/testdata/maven-projects.tar.gz
new file mode 100644
index 0000000..468ec81
Binary files /dev/null and b/testdata/maven-projects.tar.gz differ