Change approach for detecting invalid targets

Initially (7374840), snakemake-check-target only used regular
expressions to detect invalid targets based on the presence of a
MissingRuleException or RuleException in Snakemake's output.  The target
was assumed to be valid if these exceptions weren't found.  If there was
a non-zero exit status for another reason, it bubbled up to the compile
call where it was visible to the user.

33a7c90 (snakemake-check-target: Adjust for upstream output, 2016-09-01)
restricted the invalid target check to calls with an exit status of
zero.  This makes the regular expression check useless because snakemake
should always exit with a non-zero status if a MissingRuleException or
RuleException is thrown.  Due to this change, snakemake-check-target
classified all non-zero exits as invalid and all zero exits as valid.
While this often gives the right answer, it doesn't in cases where the
non-zero exit is unrelated to an invalid target.
2bceb7f (snakemake-check-target: Recognize protected items, 2016-09-05)
addressed one case.

To deal with other cases (such as an ambiguous rule error or a syntax
error in the Snakefile), use the following approach.

* An exit status of zero indicates a valid target.

* A non-zero exit status indicates an invalid target if
  snakemake-all-rules has an exit status of zero.  Otherwise,
  snakemake-all-rules will signal an error and display the Snakemake
  output.

The main downside of this approach is the need to call snakemake twice.
The output of snakmake-all-rules is cached, so this is only the case on
the first call to snakemake-check-target for a given version of a
Snakefile.

2 files changed, 34 insertions, 12 deletions

diff --git a/snakemake-test.el b/snakemake-test.el
index 80b6bcd..19a0b99 100644
--- a/snakemake-test.el
+++ b/snakemake-test.el
@@ -820,6 +820,29 @@ two words"
   (should-not
    (snakemake-with-temp-dir
      (snakemake-check-target "cc_wildcards")))
+  ;; Errors with the Snakefile, like ambiguous rules and syntax
+  ;; errors, should be reported as errors rather than treated as
+  ;; invalid targets.
+  (should-error
+   (snakemake-with-temp-dir
+     (write-region "\ndef incomplete_def:"
+                   nil
+                   "Snakefile"
+                   'append)
+     (snakemake-check-target "aa"))
+   :type 'snakemake-error)
+  (should-error
+   (snakemake-with-temp-dir
+     (write-region "\
+
+rule aa:
+    output: \"aa.ambig.out\"
+    shell: \"echo aa.ambig.content > {output}\""
+                   nil
+                   "Snakefile"
+                   'append)
+     (snakemake-check-target "aa"))
+   :type 'snakemake-error)
   ;; Write-protected targets should be recognized as valid targets
   ;; despite Snakemake throwing an error.
   (should
diff --git a/snakemake.el b/snakemake.el
index 5baeae1..33f5f5d 100644
--- a/snakemake.el
+++ b/snakemake.el
@@ -266,10 +266,6 @@ The file list is determined by the output of
              (buffer-string)
            (snakemake--display-error)))))))
 
-(defconst snakemake-invalid-target-re
-  (regexp-opt (list "MissingRuleException"
-                    "RuleException")))
-
 (defconst snakemake-valid-target-re "ProtectedOutputException"
   "Regular expression indicating valid target.
 If this matches, the target will be considered valid even if the
@@ -280,14 +276,17 @@ exit status is non-zero.")
   (snakemake-with-cache directory (target)
     (with-temp-buffer
       (let ((ex-code (snakemake-insert-output "--quiet" "--dryrun" target)))
-        (goto-char (point-min))
-        (cond
-         ((re-search-forward snakemake-valid-target-re nil t))
-         ((and (zerop ex-code)
-               ;; Lean towards misclassifying targets as valid rather
-               ;; than silently dropping valid targets as invalid.
-               (not (re-search-forward snakemake-invalid-target-re
-                                       nil t)))))))))
+        (or (zerop ex-code)
+            (progn (goto-char (point-min))
+                   (re-search-forward snakemake-valid-target-re nil t))
+            ;; A non-zero exit status could indicate that TARGET is
+            ;; invalid, but it could also be the result of an issue
+            ;; like a syntax error or an ambiguous rule.  To check
+            ;; this, see whether `snakemake-all-rules' signals a
+            ;; `snakemake-error'.  This avoids relying on parsing
+            ;; Snakemake exception output, which isn't stable across
+            ;; Snakemake versions.
+            (progn (snakemake-all-rules) nil))))))
 
 (declare-function org-element-context "org-element")
 (declare-function org-element-property "org-element")