Initial unit tests for cleardep

.classpath

@@ -2,6 +2,7 @@
 <classpath>
 	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
 	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="src" path="test"/>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
 	<classpathentry kind="output" path="bin"/>
 </classpath>

META-INF/MANIFEST.MF

@@ -9,7 +9,8 @@ Require-Bundle: org.eclipse.ui,
  org.strategoxt.strj,
  org.spoofax.jsglr,
  org.eclipse.jdt.core,
- org.spoofax.terms
+ org.spoofax.terms,
+ org.junit
 Bundle-ActivationPolicy: lazy
 Bundle-RequiredExecutionEnvironment: JavaSE-1.7
 Export-Package: org.sugarj.common,

build.properties

@@ -1,4 +1,5 @@
-source.. = src/
+source.. = src/,\
+           test/
 output.. = bin/
 bin.includes = META-INF/,\
                .

src/org/sugarj/common/cleardep/CompilationUnit.java

@@ -21,7 +21,11 @@
 import org.sugarj.util.Pair;
 
 /**
- * Dependency management for modules.
+ * Dependency management for modules. <br>
+ * <br>
+ * For each module there are two <i>CompilationUnit</i>s. One for automatic
+ * compilations (generates only temporary files) and one for "real" compilation
+ * that may use already generated files (if valid)
  * 
  * @author Sebastian Erdweg
  */
@@ -37,17 +41,55 @@ public CompilationUnit() { /* for deserialization only */ }
 
   protected Synthesizer syn;
 
+  /**
+   * if this == compiledCompilationUnit -> temporary directory <br>
+   * if this == editedCompilationUnit   -> i.e. /bin folder
+   */
   protected Path targetDir;
+
+  /**
+   * source files
+   */
   protected Map<RelativePath, Integer> sourceArtifacts;
+
   protected Set<CompilationUnit> moduleDependencies;
-  protected Set<CompilationUnit> circularModuleDependencies;  
+  protected Set<CompilationUnit> circularModuleDependencies;
+
   protected Map<Path, Integer> externalFileDependencies;
+
   protected Map<Path, Integer> generatedFiles;
 
   // **************************
   // Methods for initialization
   // **************************
 
+  /**
+   * 
+   * @param cl
+   * @param stamper
+   * @param compileDep
+   *          path to the persisted compiledCompilationUnit (.dep file)
+   * @param compileTarget
+   *          i.e.: /bin folder
+   * @param editedDep
+   *          path to the persisted editedCompilationUnit (.dep file)
+   * @param editedTarget
+   *          i.e.: some temporary folder
+   * @param sourceFiles
+   * @param editedSourceFiles
+   *          is empty or null, except source has been edited
+   * @param mode
+   * 
+   * @param syn
+   *          == null, except for generated modules (which are a product of a
+   *          transformation on another module)
+   * @return <br>
+   *         <ul>
+   *         <li>edited mode -> editedCompilationUnit
+   *         <li>compiled mode -> compiledCompilationUnit
+   *         </ul>
+   * @throws IOException
+   */
   @SuppressWarnings("unchecked")
   final protected static <E extends CompilationUnit> E create(Class<E> cl, Stamper stamper, Path compileDep, Path compileTarget, Path editedDep, Path editedTarget, Set<RelativePath> sourceFiles, Map<RelativePath, Integer> editedSourceFiles, Mode mode, Synthesizer syn) throws IOException {
     E compileE;
@@ -90,6 +132,22 @@ final protected static <E extends CompilationUnit> E create(Class<E> cl, Stamper
     return e;
   }
 
+  /**
+   * a) mode == DoCompileMode (true) checks all files in the compileTarget
+   * folder (i.e. /bin) and all other dependent modules for consistency
+   * 
+   * @param cl
+   * @param stamper
+   * @param compileDep
+   *          path to the persisted compiledCompilationUnit (.dep file)
+   * @param editedDep
+   *          path to the persisted editedCompilationUnit (.dep file)
+   * @param editedSourceFiles
+   *          is empty / null, except source has been edited
+   * @param mode
+   * @return
+   * @throws IOException
+   */
   @SuppressWarnings("unchecked")
   final protected static <E extends CompilationUnit> Pair<E, Boolean> read(Class<E> cl, Stamper stamper, Path compileDep, Path editedDep, Map<RelativePath, Integer> editedSourceFiles, Mode mode) throws IOException {
     E compileE = PersistableEntity.read(cl, stamper, compileDep);
@@ -116,9 +174,15 @@ final protected static <E extends CompilationUnit> Pair<E, Boolean> read(Class<E
     return Pair.create(DoCompileMode.isDoCompile(mode) ? compileE : editedE, false);
   }
 
+/**
+ * Copies content of a (consistent) <i>editedCompilationUnit</i> to a <i>compiledCompilationUnit</i>
+ * @param compiled
+ */
   protected void copyContentTo(CompilationUnit compiled) {
     compiled.sourceArtifacts.putAll(sourceArtifacts);
 
+    // TODO: shouldn't the synthesizer also be copied?
+
     for (CompilationUnit dep : moduleDependencies)
       if (dep.compiledCompilationUnit == null)
         compiled.addModuleDependency(dep);
@@ -138,6 +202,10 @@ protected void copyContentTo(CompilationUnit compiled) {
       compiled.addGeneratedFile(FileCommands.tryCopyFile(targetDir, compiled.targetDir, p));
   }
 
+  /**
+   * Copies contents of all dependent (consistent) <i>editedCompilationUnit</i>s to <i>compiledCompilationUnit</i>s
+   * @throws IOException
+   */
   protected void liftEditedToCompiled() throws IOException {
     ModuleVisitor<Void> liftVisitor = new ModuleVisitor<Void>() {
       @Override public Void visit(CompilationUnit mod, Mode mode) {
@@ -207,19 +275,40 @@ public void addGeneratedFile(Path file, int stampOfFile) {
     generatedFiles.put(file, stampOfFile);
   }
 
+  /**
+   * use addModuleDependency(...) instead
+   */
+  @Deprecated
   public void addCircularModuleDependency(CompilationUnit mod) {
     circularModuleDependencies.add(mod);
   }
 
-  public void addModuleDependency(CompilationUnit mod) {
+  /**
+   * @return true if mod was added to moduleDependencies <br>
+   *         false if mod was added to circularModuleDependencies
+   */
+  public boolean addModuleDependency(final CompilationUnit mod) {
+
+    if (mod.dependsOnTransitively(this) || this.dependsOnTransitively(mod)) {
+
+      circularModuleDependencies.add(mod);
+      return false;
+    }
+
     moduleDependencies.add(mod);
+    return true;
+
   }
 
 
   // *********************************
   // Methods for querying dependencies
   // *********************************
 
+  /**
+   * is edited ...
+   * @return
+   */
   public boolean isParsedCompilationUnit() {
     return compiledCompilationUnit != null;
   }
@@ -237,16 +326,23 @@ public boolean dependsOnTransitivelyNoncircularly(CompilationUnit other) {
     return false;
   }
 
+  /**
+   * Depends on directly
+   * @param other
+   * @return
+   */
   public boolean dependsOn(CompilationUnit other) {
     return moduleDependencies.contains(other) || circularModuleDependencies.contains(other);    
   }
 
   public boolean dependsOnTransitively(CompilationUnit other) {
+
     if (dependsOn(other))
       return true;
     for (CompilationUnit mod : moduleDependencies)
       if (mod.dependsOnTransitively(other))
         return true;
+
     return false;
   }
 
@@ -309,6 +405,12 @@ public Synthesizer getSynthesizer() {
 
   protected abstract boolean isConsistentExtend(Mode mode);
 
+  /**
+   * @param editedSourceFiles
+   * @param mode
+   *          TODO: unused?
+   * @return
+   */
   protected boolean isConsistentWithSourceArtifacts(Map<RelativePath, Integer> editedSourceFiles, Mode mode) {
     if (sourceArtifacts.isEmpty())
       return false;
@@ -325,6 +427,19 @@ else if (stamp == null && (!FileCommands.exists(e.getKey()) || e.getValue() != s
     return true;
   }
 
+  /**
+   * Checks consistency only for this module's ...
+   * <ul>
+   * <li>source files
+   * <li>generated files
+   * <li>external file dependencies
+   * </ul>
+   * 
+   * @param editedSourceFiles
+   * @param mode
+   *          (compiled / edited / ... )
+   * @return
+   */
   public boolean isConsistentShallow(Map<RelativePath, Integer> editedSourceFiles, Mode mode) {
     if (hasPersistentVersionChanged())
       return false;
@@ -346,6 +461,14 @@ public boolean isConsistentShallow(Map<RelativePath, Integer> editedSourceFiles,
     return true;
   }
 
+  /**
+   * Checks consistency of this module including its module dependencies <br>
+   * 
+   * @param editedSourceFiles
+   * @param mode
+   *          (compiled / edited / ... )
+   * @return
+   */
   public boolean isConsistent(final Map<RelativePath, Integer> editedSourceFiles, Mode mode) {
     ModuleVisitor<Boolean> isConsistentVisitor = new ModuleVisitor<Boolean>() {
       @Override public Boolean visit(CompilationUnit mod, Mode mode) { return mod.isConsistentShallow(editedSourceFiles, mode); }
@@ -411,12 +534,16 @@ private Pair<Map<CompilationUnit, Integer>, Map<CompilationUnit, Mode>> computeR
 
 
   /**
-   * Visits the module graph starting from this module, satisfying the following properties:
-   *  - every module transitively imported from `this` module is visited exactly once
-   *  - if a module M1 is visited before a module M2,
-   *    then (i) M1 is not transitively imported from M2 or 
-   *         (ii) M1 and M2 transitively have a circular dependency and
-   *              M1 transitively imports M2 using `moduleDependencies` only.   
+   * Visits the module graph starting from this module, satisfying the following
+   * properties: - every module transitively imported from `this` module is
+   * visited exactly once - if a module M1 is visited before a module M2, then
+   * (i) M1 is not transitively imported from M2 or (ii) M1 and M2 transitively
+   * have a circular dependency and M1 transitively imports M2 using
+   * `moduleDependencies` only.
+   * 
+   * Shorter: Visits every module that is transitively imported from 'this'
+   * module exactly once Visits Module M1 before M2 if <i>M1 is not transitively
+   * (-noncircularly) imported from M2</i>
    */
   public <T> T visit(ModuleVisitor<T> visitor) { return visit(visitor, null); }
   public <T> T visit(ModuleVisitor<T> visitor, Mode thisMode) {

src/org/sugarj/common/cleardep/Synthesizer.java

@@ -8,12 +8,27 @@
 import org.sugarj.common.path.Path;
 
 /**
+ * <ul>
+ * <li>Used to dependency-track modules (called A$B) that are a product of a transformation (B) on another module (A)
+ * <li>Holds references to the required modules (A and B)
+ * <li>The module that includes the transformed module has only a dependency on A$B
+ * <li>The CompilationUnit of A$B now contains this Synthesizer (called CompilationUnit.syn)
+ * </ul>
+ * 
  * @author Sebastian Erdweg
  */
 public class Synthesizer {
   public Set<CompilationUnit> modules;
-  public Map<Path, Integer> files;
+  public Map<Path, Integer> files; // external file dependencies
 
+  /**
+   * 
+   * @param modules
+   *          required by the module to be synthesized
+   * @param files
+   *          external file dependencies required by the module to be
+   *          synthesized
+   */
   public Synthesizer(Set<CompilationUnit> modules, Map<Path, Integer> files) {
     this.modules = modules;
     this.files = files;
@@ -26,10 +41,13 @@ public Synthesizer(Stamper stamper, Set<CompilationUnit> modules, Set<Path> file
       this.files.put(p, stamper.stampOf(p));
   }
 
-  public void markSynthesized(CompilationUnit c) {
+  public void markSynthesized(CompilationUnit synthesizedModule) {
     for (CompilationUnit m : modules)
-      c.addModuleDependency(m);
+      synthesizedModule.addModuleDependency(m);
+    // TODO: maybe the bug was here, when addModuleDiependency didn't recognize
+    // cycles
+
     for (Entry<Path, Integer> e : files.entrySet())
-      c.addExternalFileDependency(e.getKey(), e.getValue());
+      synthesizedModule.addExternalFileDependency(e.getKey(), e.getValue());
   }
 }