Permalink
Browse files

Precedence cycle detected implemented.

  • Loading branch information...
1 parent 855f0c8 commit 85369c6880cae63d0947ff75f318caa2536ef8ee @aroemers committed Nov 9, 2012
Showing with 99 additions and 12 deletions.
  1. +5 −3 README.md
  2. +4 −0 src-java/test/bind.gluer
  3. +5 −2 src/gluer/core.clj
  4. +85 −7 src/gluer/logic.clj
View
@@ -133,8 +133,9 @@ To have an idea which Adapter will be chosen at runtime (and how resolution conf
* If more than one has been found, continue to step 4.
-4. Multiple Adapters are equally close. Now the tool looks at the declared precendences, if any. All equally close Adapters are filtered, by checking whether they are preceded by another Adapter in the list. If so, it will be removed from the list (It is removed at the end of this filtering process, so circular precedence rules would yield an empty list. Adding a check for circular precedence declarations is a future improvement).
+4. Multiple Adapters are equally close. Now the tool looks at the declared precendences, if any. All equally close Adapters are filtered, by checking whether they are preceded by another Adapter in the list. If so, it will be removed from the list (It is removed at the end of this filtering process, so circular precedence rules would yield an empty list).
* If this way the list has shrunk to one, we are done and that Adapter will be used. We continue at step 5.
+ * If the list is empty, this signifies circular precedence declarations. These circular precedence declarations are already reported as warnings, by the checker, _before_ adapter resolution. If it actually occurs _during_ adapter resolution, it reports an error (or throws a RuntimeException).
* If, however, the list still contains two or more Adapters, the framework cannot make a decision which Adapter to use and reports a resolution conflict error (or throws a RuntimeException).
@@ -172,7 +173,9 @@ Another way of resolving resolution conflicts, is to declare precedence rules. F
declare precedence PreferredAdapter over PrecededAdapter
```
-Precedence declarations and association statements can be mixed in a .gluer file, and their order does not matter.
+In _checking_ mode, circular precedence declarations are detected and reported as warnings. If, however, an association statement may be hindered by these circular precedence declarations, an error is reported. Note that not all such errors can be detected statically, so it is good to pay attention to the warnings.
+
+Precedence declarations and association statements can be mixed in a .gluer file, and their order does not matter.
An example .gluer file:
@@ -219,7 +222,6 @@ verbose: false
The following points are possible future improvements. In no particular order:
* Generics and typed collections support. E.g. adapt a List\<Foo> to List\<Bar> by injecting a List\<Foo2Bar>
-* Statically check for circular precedence declarations
* Statically check for possible resolution conflicts occuring during run-time due to actual subtypes of _what_ is injected.
* Improved test suite, proving the correct implementation of implicit rules (such as adapter resolution) and correct coverage of static checks (such as detecting resolution conflicts)
* Injection in static fields
View
@@ -7,4 +7,8 @@ associate field test.Main.superSuperA with single test.modelb.SubSubB using test
declare precedence test.adapter.TwiceBtoA1 over test.adapter.TwiceBtoA2
+declare precedence test.adapter.TwiceBtoA2 over test.adapter.TwiceBtoA3
+
+declare precedence test.adapter.TwiceBtoA3 over test.adapter.TwiceBtoA1
+
associate field test.Main.twice with new test.modelb.TwiceB
View
@@ -67,9 +67,9 @@
(let [warnings (:warnings value)
errors (:errors value)]
(when (not (empty? warnings))
- (print-issues (if (seq? warnings) warnings (list warnings)) :warning))
+ (print-issues (if (coll? warnings) warnings (list warnings)) :warning))
(when (not (empty? errors))
- (print-issues (if (seq? errors) errors (list errors)) :error)
+ (print-issues (if (coll? errors) errors (list errors)) :error)
(throw (InterruptedException.))))
value)
@@ -97,6 +97,9 @@
_ (log-verbose "Building precedence relations...")
precedence-relations (r/build-precedence-relations parsed-precedences)
_ (log-verbose "Precedence relations:" precedence-relations)
+ _ (log-verbose "Checking precedence relations for cycles...")
+ _ (do-check (l/check-precedence-relations precedence-relations))
+
_ (log-verbose "Checking associations...")
adapter-library (assoc adapter-library :precedence precedence-relations)
parsed-associations (r/parsed-associations parsed-files)
View
@@ -28,6 +28,10 @@
"\tAdd a 'using' clause to the association, or declare precedence rules "
"for the closest adapters."))
+(def precedence-error
+ (str "Resolution conflict due to cyclic precedence declarations, see warnings above.\n"
+ "\tConflicting adapters are: %s."))
+
(def not-found-error
"Adapter %s is not found. Make sure it is spelled correctly and is in the classpath.")
@@ -114,12 +118,15 @@
(let [precedence-relations (:precedence adapter-library)
closest-names (set (keys closest))
preferred (remove #(some closest-names (precedence-relations %)) closest-names)]
- (if (= 1 (count preferred))
- {:result (first preferred)}
- {:error (format resolution-conflict-error from-name to-name
- (apply str (interpose ", " preferred))
- (apply str (interpose ", " closest-names))
- (apply str (interpose ", " (keys eligible))))})))))))
+ (cond
+ (empty? preferred)
+ {:error (format precedence-error (apply str (interpose ", " closest-names)))}
+ (= 1 (count preferred))
+ {:result (first preferred)}
+ :otherwise
+ {:error (format resolution-conflict-error from-name to-name
+ (apply str (interpose ", " preferred))
+ (apply str (interpose ", " closest-names)))})))))))
;;; Checking utilities.
@@ -282,7 +289,7 @@
(map #(format-issue % file-name (line-nr precedence)))
(hash-map :errors))))
-(defn check-precedences ;--- TODO: Check for circular precedence declarations.
+(defn check-precedences
"Given a collection of file-precedence pairs, as returned by
`gluer.resources/parsed-precedences', it will check each precedence
declaration. The function returns a map in the following form:
@@ -296,6 +303,77 @@
;; Merge the {:errors [..] :warnings [..]} maps.
(reduce (partial merge-with concat) precedence-check-results)))
+(defn- check-precedence-cycle ;--- It was easier to write this specific code, than to use/create a
+ ; graph library. Maybe fix this when other graphs are needed.
+ "Given a map with precedence relations, a starting key (name) and a set
+ holding the current path (initially empty), the function recursively checks
+ whether it can detect a cycle. The function returns a pair, where the first
+ item is the list of keys it has visited, and the second item is a boolean
+ indicating whether there was a cycle in visiting those keys.
+
+ For instance, when called with the following arguments:
+
+ precedence-relations: {\"A\" #{\"B\"}
+ \"B\" #{\"C\"}}
+ name: \"A\"
+ path: #{}
+
+ It would return:
+
+ [(\"A\" \"B\") false]
+
+ If no cycle was found, the full tree has been visited. Otherwise, part of the
+ tree may be unvisited. Note that the precedence-relations may consist of
+ multiple trees."
+ [precedence-relations name path]
+ ;; Check if node (name) has 'edges' to other nodes.
+ (if-let [precedes (precedence-relations name)]
+ ;; Loop through edges.
+ (loop [ps precedes
+ visited []]
+ (if-let [p (first ps)]
+ ;; Check if the node on the other end of the edge has already been visited.
+ (if (path p)
+ ;; Already visisted, cycle detected. Stop here.
+ [[name] true]
+ ;; Not visited yet, recursively check it for cycles.
+ (let [[visit cycle?] (check-precedence-cycle precedence-relations p (conj path name))]
+ ;; Check if cycle was detected.
+ (if cycle?
+ ;; Cycles detected, stop here.
+ [(conj (concat visit visited) name) true]
+ ;; No cycle detected yet, continue to visit other edges.
+ (recur (rest ps) (concat visit visited)))))
+ ;; Done visisting edges, no cycle detected.
+ [(conj visited name) false]))
+ ;; No edges found, consider this node not visited and no cycle detected.
+ [[] false]))
+
+(defn check-precedence-relations
+ "Given a map with precedence relations (as returned by
+ `gluer.resources/build-precedence-relations'), this function checks if there
+ are cycles in these precedence relations. The function returns the detected
+ cycle messages in a collection in a map under the :warnings key:
+
+ {:warnings (\"Cycle detected ..\" \"Cycle detected ..\" ...)}"
+ [precedence-relations]
+ ;; Loop through all 'trees' of precedence relations. Start with the entire 'forest' and 0 warnings.
+ (loop [prels precedence-relations
+ warnings []]
+ ;; Check if any tree is left unchecked.
+ (if (empty? prels)
+ ;; All trees checked, returns accumulated warnings.
+ {:warnings warnings}
+ ;; Some tree(s) unchecked, check the current graph for cycles, starting from some random node.
+ (let [[visited cycle?] (check-precedence-cycle prels (first (keys prels)) #{})]
+ ;; Remove visited nodes from graph, accumulate a warning if a cycle was detected and loop.
+ (recur (apply dissoc prels visited)
+ (if cycle?
+ (conj warnings
+ (str "Cycle detected in precedence relations for adapters: "
+ (apply str (interpose ", " visited)) "."))
+ warnings))))))
+
(defn check-parse-results
"Given the parse results by `gluer.resources/parse-gluer-files', this checks
if some of the files failed to parse. The function returns a map in the

0 comments on commit 85369c6

Please sign in to comment.