diff --git a/src/main/java/com/github/sommeri/less4j/core/compiler/expressions/ExpressionsEvaluator.java b/src/main/java/com/github/sommeri/less4j/core/compiler/expressions/ExpressionsEvaluator.java index 13d8d28f..3d72e6d5 100644 --- a/src/main/java/com/github/sommeri/less4j/core/compiler/expressions/ExpressionsEvaluator.java +++ b/src/main/java/com/github/sommeri/less4j/core/compiler/expressions/ExpressionsEvaluator.java @@ -1,7 +1,6 @@ package com.github.sommeri.less4j.core.compiler.expressions; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.Stack; @@ -40,6 +39,7 @@ import com.github.sommeri.less4j.core.compiler.scopes.IScope; import com.github.sommeri.less4j.core.compiler.scopes.NullScope; import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory; +import com.github.sommeri.less4j.core.compiler.stages.CallerCalleeScopeJoiner; import com.github.sommeri.less4j.core.problems.BugHappened; import com.github.sommeri.less4j.core.problems.ProblemsHandler; import com.github.sommeri.less4j.utils.CssPrinter; @@ -51,6 +51,7 @@ public class ExpressionsEvaluator { private final IScope lazyScope; private final Stack eagerScopes = new Stack(); private final ProblemsHandler problemsHandler; + private final CallerCalleeScopeJoiner scopesJoiner = new CallerCalleeScopeJoiner(); private ArithmeticCalculator arithmeticCalculator; private ColorsCalculator colorsCalculator; @@ -153,11 +154,11 @@ private Expression evaluate(Variable input, boolean failOnUndefined) { return handleUndefinedVariable(input, failOnUndefined); } - IScope originalScope = enteringScopeOf(expression); + IScope addedScope = enteringScopeOf(expression); cycleDetector.enteringVariableValue(input); Expression result = evaluate(expression); cycleDetector.leftVariableValue(); - leavingScope(originalScope); + leavingScope(addedScope); return result; } @@ -287,7 +288,6 @@ 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 splitParameters = (evaluatedParameter.getType() == ASTCssNodeType.EMPTY_EXPRESSION) ? new ArrayList() : evaluatedParameter.splitByComma(); @@ -418,36 +418,25 @@ public Expression evaluate(DetachedRuleset input) { return clone; } - //FIXME !!!!!!! do something smarter, if they are local to each other they need special handling - //FIXME !!!!!!! make sure detached ruleset does not tie its own scope twice to itself private IScope composedScope(IScope owningScope) { - //FIXME!!!!!!!!!!! nicer design, this whole method could be inside scope factory - Iterator nextEager = eagerScopes.iterator(); - if (!nextEager.hasNext()) - return owningScope; - - IScope result = nextEager.next(); - while (nextEager.hasNext()) { - //FIXME!!!!!!!!!!! chech whether joining needs to happen - e.g. whehter they see each other - IScope next = nextEager.next(); - result = ScopeFactory.createJoinedScopesView(result, next); - } - result = ScopeFactory.createJoinedScopesView(result, owningScope); - - return result; + return scopesJoiner.createJoinedScopes(eagerScopes, owningScope); } - private void leavingScope(IScope originalScope) { - if (originalScope != null) { + private void leavingScope(IScope addedScope) { + if (addedScope != null) { eagerScopes.pop(); } } private IScope enteringScopeOf(Expression value) { IScope owningScope = value.getScope(); - if (owningScope != null) { - eagerScopes.push(owningScope); - } + if (owningScope==null) + return null; + + if (!eagerScopes.isEmpty() && eagerScopes.peek()==eagerScopes) + return null; + + eagerScopes.push(owningScope); return owningScope; } diff --git a/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/BasicScope.java b/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/BasicScope.java index be74e7ee..c210314d 100644 --- a/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/BasicScope.java +++ b/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/BasicScope.java @@ -62,7 +62,8 @@ public boolean seesLocalDataOf(IScope otherScope) { return getParent().seesLocalDataOf(otherScope); } - //FIXME ! this method might be wasteful, we are looking for the same tree, so there should be no need to restart search from top for each parent + // this method might be wasteful, we are looking for the same tree, so there should + // be no need to restart search from top for each parent public boolean seesAllDataOf(IScope otherScope) { boolean isLocalImport = seesLocalDataOf(otherScope); IScope parent = otherScope.getParent(); diff --git a/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/ScopeManipulation.java b/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/ScopeManipulation.java deleted file mode 100644 index 0bf6c60a..00000000 --- a/src/main/java/com/github/sommeri/less4j/core/compiler/scopes/ScopeManipulation.java +++ /dev/null @@ -1,83 +0,0 @@ -package com.github.sommeri.less4j.core.compiler.scopes; - -import java.util.ArrayList; -import java.util.List; - -import com.github.sommeri.less4j.core.compiler.scopes.view.ScopeView; - -public class ScopeManipulation { - - //FIXME !!!! refactor and clean, unify with references solver joiner - public ScopeView constructImportedBodyScope(IScope importTargetScope, IScope bodyToBeImportedScope) { - ScopeView newScope = null; - // locally defined mixin does not require any other action - boolean isLocalImport = bodyToBeImportedScope.seesAllDataOf(importTargetScope); - - if (isLocalImport) { - // we need to copy the whole tree, because this runs inside referenced mixin scope - // snapshot and imported mixin needs to remember the scope as it is now - newScope = ScopeFactory.createJoinedScopesView(null, bodyToBeImportedScope); - newScope.saveLocalDataForTheWholeWayUp(); - } else { - // since this is non-local import, we need to join reference scope and imported mixins scope - // imported mixin needs to have access to variables defined in caller - newScope = ScopeFactory.createJoinedScopesView(importTargetScope, bodyToBeImportedScope); - newScope.saveLocalDataForTheWholeWayUp(); - } - return newScope; - } - - public IScope calculateBodyWorkingScope(IScope callerScope, IScope bodyScope) { - //FIXME !!!!!! find the case where this is needed for mixins and create test case for it - // locally defined mixin does not require any other action - boolean isLocallyDefined = bodyScope.seesAllDataOf(callerScope); - - if (isLocallyDefined) { - return bodyScope; - } - - //join scopes - IScope result = ScopeFactory.createJoinedScopesView(callerScope, bodyScope); - return result; - } - - //FIXME: !!! evaluate need for this - public IScope calculateMixinsWorkingScope(IScope callerScope, IScope arguments, IScope mixinScope) { - // add arguments - IScope mixinDeclarationScope = mixinScope.getParent(); - mixinDeclarationScope.add(arguments); - - // locally defined mixin does not require any other action - boolean isLocallyDefined = mixinDeclarationScope.seesLocalDataOf(callerScope); - if (isLocallyDefined) { - return mixinScope; - } - - //join scopes - IScope result = ScopeFactory.createJoinedScopesView(callerScope, mixinScope); - return result; - } - - public List mixinsToImport(IScope referenceScope, IScope referencedMixinScope, List unmodifiedMixinsToImport) { - List result = new ArrayList(); - for (FullMixinDefinition mixinToImport : unmodifiedMixinsToImport) { - boolean isLocalImport = mixinToImport.getScope().seesLocalDataOf(referenceScope); - if (isLocalImport) { - // we need to copy the whole tree, because this runs inside referenced mixin scope - // snapshot and imported mixin needs to remember the scope as it is now - ScopeView newWay = ScopeFactory.createJoinedScopesView(null, mixinToImport.getScope()); - newWay.saveLocalDataForTheWholeWayUp(); - result.add(new FullMixinDefinition(mixinToImport.getMixin(), newWay)); - } else { - // since this is non-local import, we need to join reference scope and imported mixins scope - // imported mixin needs to have access to variables defined in caller - ScopeView newWay = ScopeFactory.createJoinedScopesView(referencedMixinScope, mixinToImport.getScope()); - newWay.saveLocalDataForTheWholeWayUp(); - result.add(new FullMixinDefinition(mixinToImport.getMixin(), newWay)); - } - - } - return result; - } - -} diff --git a/src/main/java/com/github/sommeri/less4j/core/compiler/stages/CallerCalleeScopeJoiner.java b/src/main/java/com/github/sommeri/less4j/core/compiler/stages/CallerCalleeScopeJoiner.java new file mode 100644 index 00000000..17515278 --- /dev/null +++ b/src/main/java/com/github/sommeri/less4j/core/compiler/stages/CallerCalleeScopeJoiner.java @@ -0,0 +1,84 @@ +package com.github.sommeri.less4j.core.compiler.stages; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import com.github.sommeri.less4j.core.compiler.scopes.FullMixinDefinition; +import com.github.sommeri.less4j.core.compiler.scopes.IScope; +import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory; +import com.github.sommeri.less4j.core.compiler.scopes.view.ScopeView; + +public class CallerCalleeScopeJoiner { + + public ScopeView joinIfIndependentAndPreserveContent(IScope callerScope, IScope bodyScope) { + // locally defined mixin does not require any other action + boolean isLocalImport = bodyScope.seesLocalDataOf(callerScope); + + ScopeView result = null; + if (isLocalImport) { + // we need to copy the whole tree, because this runs inside referenced mixin scope + // snapshot and imported mixin needs to remember the scope as it is now + result = ScopeFactory.createJoinedScopesView(null, bodyScope); + } else { + // since this is non-local import, we need to join reference scope and imported mixins scope + // imported mixin needs to have access to variables defined in caller + result = ScopeFactory.createJoinedScopesView(callerScope, bodyScope); + } + result.saveLocalDataForTheWholeWayUp(); + return result; + } + + public IScope joinIfIndependent(IScope callerScope, IScope bodyScope) { + // locally defined mixin does not require any other action + boolean isLocallyDefined = bodyScope.seesLocalDataOf(callerScope); + + if (isLocallyDefined) { + return bodyScope; + } + + //join scopes + IScope result = ScopeFactory.createJoinedScopesView(callerScope, bodyScope); + return result; + } + + public List mixinsToImport(IScope callerScope, IScope calleeScope, List unmodifiedMixinsToImport) { + List result = new ArrayList(); + for (FullMixinDefinition mixinToImport : unmodifiedMixinsToImport) { + boolean isLocalImport = mixinToImport.getScope().seesLocalDataOf(callerScope); + ScopeView newScope = null; + if (isLocalImport) { + // we need to copy the whole tree, because this runs inside referenced mixin scope + // snapshot and imported mixin needs to remember the scope as it is now + newScope = ScopeFactory.createJoinedScopesView(null, mixinToImport.getScope()); + newScope.saveLocalDataForTheWholeWayUp(); + } else { + // since this is non-local import, we need to join reference scope and imported mixins scope + // imported mixin needs to have access to variables defined in caller + newScope = ScopeFactory.createJoinedScopesView(calleeScope, mixinToImport.getScope()); + newScope.saveLocalDataForTheWholeWayUp(); + } + + result.add(new FullMixinDefinition(mixinToImport.getMixin(), newScope)); + } + return result; + } + + public IScope createJoinedScopes(List parents, IScope child) { + // this could and maybe should check whether they are local to each other e.g., whther they see each other + // if they are local, parent scope could be skipped + // such logic is not implemented yet + Iterator iterator = parents.iterator(); + if (!iterator.hasNext()) + return child; + + IScope result = iterator.next(); + while (iterator.hasNext()) { + IScope next = iterator.next(); + result = ScopeFactory.createJoinedScopesView(result, next); + } + result = ScopeFactory.createJoinedScopesView(result, child); + + return result; + } +} diff --git a/src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsSolver.java b/src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsSolver.java index 849ae6b8..27b2e58b 100644 --- a/src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsSolver.java +++ b/src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsSolver.java @@ -24,7 +24,6 @@ import com.github.sommeri.less4j.core.compiler.scopes.InScopeSnapshotRunner.IFunction; import com.github.sommeri.less4j.core.compiler.scopes.InScopeSnapshotRunner.ITask; import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory; -import com.github.sommeri.less4j.core.compiler.scopes.ScopeManipulation; import com.github.sommeri.less4j.core.problems.ProblemsHandler; class MixinsSolver { @@ -34,7 +33,7 @@ class MixinsSolver { private final AstNodesStack semiCompiledNodes; private final Configuration configuration; private final DefaultGuardHelper defaultGuardHelper; - private final ScopeManipulation scopeManipulation = new ScopeManipulation(); + private final CallerCalleeScopeJoiner scopeManipulation = new CallerCalleeScopeJoiner(); public MixinsSolver(ReferencesSolver parentSolver, AstNodesStack semiCompiledNodes, ProblemsHandler problemsHandler, Configuration configuration) { this.parentSolver = parentSolver; @@ -111,8 +110,10 @@ public GeneralBody buildMixinReferenceReplacement(final MixinReference reference @Override public void run() { + // add arguments IScope mixinArguments = buildMixinsArguments(reference, callerScope, fullMixin); - IScope mixinWorkingScope = scopeManipulation.calculateMixinsWorkingScope(callerScope, mixinArguments, mixinScope); + mixinScope.getParent().add(mixinArguments); + IScope mixinWorkingScope = scopeManipulation.joinIfIndependent(callerScope, mixinScope); MixinsGuardsValidator guardsValidator = new MixinsGuardsValidator(mixinWorkingScope, problemsHandler, configuration); GuardValue guardValue = guardsValidator.evaluateGuards(mixin); @@ -147,8 +148,7 @@ public void run() { } public GeneralBody buildDetachedRulesetReplacement(DetachedRulesetReference reference, IScope callerScope, DetachedRuleset detachedRuleset, IScope detachedRulesetScope) { - //FIXME: !!!!!!!!!!!! this should run in detachedRulesetScope parent snapshot - check whether it is the case - IScope mixinWorkingScope = scopeManipulation.calculateBodyWorkingScope(callerScope, detachedRulesetScope); + IScope mixinWorkingScope = scopeManipulation.joinIfIndependent(callerScope, detachedRulesetScope); BodyCompilationResult compiled = resolveCalledBody(callerScope, detachedRuleset, mixinWorkingScope); GeneralBody result = new GeneralBody(reference.getUnderlyingStructure()); @@ -181,12 +181,11 @@ private void declarationsAreImportant(Body result) { } } - //FIXME !!!! refactor and clean, unify with references class ImportedScopeFilter implements ExpressionFilter { private final ExpressionsEvaluator expressionEvaluator; private final IScope importTargetScope; - private final ScopeManipulation scopeManipulation = new ScopeManipulation(); + private final CallerCalleeScopeJoiner scopeManipulation = new CallerCalleeScopeJoiner(); public ImportedScopeFilter(ExpressionsEvaluator expressionEvaluator, IScope importTargetScope) { super(); @@ -196,7 +195,6 @@ public ImportedScopeFilter(ExpressionsEvaluator expressionEvaluator, IScope impo public Expression apply(Expression input) { Expression result = expressionEvaluator.evaluate(input); - //FIXME: !!!!!!!! probably not necessary? expression evaluator should take care of this IScope newScope = apply(result.getScope()); result.setScope(newScope); return result; @@ -206,7 +204,7 @@ private IScope apply(IScope input) { if (input == null) return importTargetScope; - return scopeManipulation.constructImportedBodyScope(importTargetScope, input); + return scopeManipulation.joinIfIndependentAndPreserveContent(importTargetScope, input); } }