Unified and cleaned up variables evaluations (direct, indirect, if pr…

…esent). Plus some minor clean ups. #186

src/main/java/com/github/sommeri/less4j/core/compiler/expressions/ExpressionsEvaluator.java

@@ -135,69 +135,41 @@ public Expression evaluate(EmbeddedScript input) {
   }
 
   public Expression evaluate(Variable input) {
+    return evaluate(input, true);
+  }
+
+  public Expression evaluateIfPresent(Variable input) {
+    return evaluate(input, false);
+  }
+
+  private Expression evaluate(Variable input, boolean failOnUndefined) {
     if (cycleDetector.wouldCycle(input)) {
       problemsHandler.variablesCycle(cycleDetector.getCycleFor(input));
       return new FaultyExpression(input);
     }
 
     Expression expression = lazyScope.getValue(input);
     if (expression == null) {
-      problemsHandler.undefinedVariable(input);
-      return new FaultyExpression(input);
-    }
-    //FIXME!!!!!!!!!!! while it is clear what should this here do once this is done, this case might 
-    //need some adjuctments while import is not fully solved   
-    if (expression.getType() == ASTCssNodeType.DETACHED_RULESET && expression.getScope() == null) {
-      throw new BugHappened("Scope information is missing", expression);
+      return handleUndefinedVariable(input, failOnUndefined);
     }
 
-    IScope originalScope = enteringExpression(expression);
+    IScope originalScope = enteringScopeOf(expression);
     cycleDetector.enteringVariableValue(input);
     Expression result = evaluate(expression);
     cycleDetector.leftVariableValue();
-    leavingExpression(originalScope);
+    leavingScope(originalScope);
     return result;
   }
 
-  private void leavingExpression(IScope originalScope) {
-    if (originalScope != null) {
-      eagerScopes.pop();
-    }
-  }
-
-  private IScope enteringExpression(Expression value) {
-    IScope owningScope = value.getScope();
-    if (owningScope != null) {
-      eagerScopes.push(owningScope);
-    }
-    return owningScope;
-  }
-
-  //FIXME: !!!!!!!!!! try this on variable that depends on undefined
-  public Expression evaluateIfPresent(Variable input) {
-    Expression value = lazyScope.getValue(input);
-    if (value == null) {
-      return null;
-    }
-
-    //FIXME !!!!!!!!!!!!!!!!! add to eager scopes
-    return evaluate(value);
-  }
-
   public Expression evaluate(IndirectVariable input) {
     Expression reference = evaluate(lazyScope.getValue(input));
 
     CssPrinter printer = new InStringCssPrinter();
     printer.append(reference);
     String realName = printer.toString();
 
-    String realVariableName = "@" + realName;
-    Expression value = lazyScope.getValue(realVariableName);
-    if (value == null) {
-      problemsHandler.undefinedVariable(realVariableName, input);
-      return new FaultyExpression(input.getUnderlyingStructure());
-    }
-    return evaluate(value);
+    Variable realVariable = new Variable(input.getUnderlyingStructure(), "@" + realName);
+    return evaluate(realVariable);
   }
 
   public Expression evaluate(Expression input) {
@@ -318,17 +290,17 @@ private boolean canCompareDimensions(Dimension left, Dimension right) {
 
   public Expression evaluate(FunctionExpression input) {
     // FIXME: !!!!!!!!!!! input parameter has null scope <- use closes parental scope if the current scope is null
-    Expression evaluatedParameter = evaluate(input.getParameter()); 
-    List<Expression> splitParameters = (evaluatedParameter.getType()==ASTCssNodeType.EMPTY_EXPRESSION)?new ArrayList<Expression>() : evaluatedParameter.splitByComma();
-    
-    if (!input.isCssOnlyFunction()) { 
+    Expression evaluatedParameter = evaluate(input.getParameter());
+    List<Expression> splitParameters = (evaluatedParameter.getType() == ASTCssNodeType.EMPTY_EXPRESSION) ? new ArrayList<Expression>() : evaluatedParameter.splitByComma();
+
+    if (!input.isCssOnlyFunction()) {
       for (FunctionsPackage pack : functions) {
         if (pack.canEvaluate(input, splitParameters))
           return pack.evaluate(input, splitParameters, evaluatedParameter);
       }
     }
-    
-    UnknownFunction unknownFunction = new UnknownFunction(); 
+
+    UnknownFunction unknownFunction = new UnknownFunction();
     return unknownFunction.evaluate(splitParameters, problemsHandler, input, evaluatedParameter);
   }
 
@@ -468,4 +440,27 @@ private IScope composedScope(IScope owningScope) {
     return result;
   }
 
+  private void leavingScope(IScope originalScope) {
+    if (originalScope != null) {
+      eagerScopes.pop();
+    }
+  }
+
+  private IScope enteringScopeOf(Expression value) {
+    IScope owningScope = value.getScope();
+    if (owningScope != null) {
+      eagerScopes.push(owningScope);
+    }
+    return owningScope;
+  }
+
+  private Expression handleUndefinedVariable(Variable variable, boolean failOnUndefined) {
+    if (failOnUndefined) {
+      problemsHandler.undefinedVariable(variable);
+      return new FaultyExpression(variable);
+    } else {
+      return null;
+    }
+  }
+
 }

src/main/java/com/github/sommeri/less4j/core/compiler/selectors/SimpleSelectorComparator.java

@@ -182,7 +182,6 @@ public boolean contains(SimpleSelector lookFor, SimpleSelector inside) {
 
   public SimpleSelector[] splitOn(SimpleSelector lookFor, SimpleSelector inside) {
     if (hasNoElement(lookFor)) {
-      //FIXME: (!!!!) test na tento flow!!!
       List<ElementSubsequent> subsequents = inside.getSubsequent();
       HiddenTokenAwareTree underlying = inside.getUnderlyingStructure();
 

src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsSolver.java

@@ -318,7 +318,6 @@ private ScopeView constructImportedBodyScope(IScope importTargetScope, IScope bo
       IScope parent = importTargetScope.getParent();
       while (isLocalImport && parent != null) {
         isLocalImport = bodyToBeImportedScope.seesLocalDataOf(parent);
-        ;
         parent = parent.getParent();
       }
 

src/main/java/com/github/sommeri/sourcemap/SourceMapGenerator.java

@@ -60,7 +60,7 @@ void appendIndexMapTo(
    * Adds a mapping for the given node.  Mappings must be added in order.
    * @param sourceName The file name to use in the generate source map
    *     to represent this source.
-   * @param sourceContent  TODO FIXME !!!!!!!!!!!!!!!!
+   * @param sourceContent  Content of the sourceName file. Parameter is optional, can be <code>null</code>.
    * 
    * @param symbolName The symbol name associated with this position in the
    *     source map.

src/test/java/com/github/sommeri/less4j/compiler/DetachedRulesetsTest.java

@@ -8,6 +8,7 @@
 /*
  * TODO: test if @defaults works correctly -- e.g. including various callers scopes
 *  TODO: test order detached mixin imports who sees who and who overwrites who
+*  FIXME: !!!!!!!!!!!!! test mixin returned from detached ruleset
  */
 public class DetachedRulesetsTest extends BasicFeaturesTest {