Skip to content

Commit

Permalink
Fixes #3169. Emits warnings for parameters with the same display name
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexisDrogoul
AlexisDrogoul committed Aug 20, 2021
1 parent 09bd85c commit 2af799f
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 102 deletions.
2 changes: 1 addition & 1 deletion msi.gama.core/src/msi/gama/kernel/experiment/ExperimentParameter.java
View file
Expand Up @@ -95,7 +95,7 @@
name = IKeyword.VAR,
type = IType.ID,
optional = false,
doc = @doc ("the name of the variable (that should be declared in the global)")),
doc = @doc ("the name of the variable (that should be declared in global)")),
@facet (
name = IKeyword.UNIT,
type = IType.LABEL,
Expand Down
134 changes: 78 additions & 56 deletions msi.gama.core/src/msi/gaml/descriptions/ExperimentDescription.java
View file
Expand Up @@ -10,10 +10,15 @@
********************************************************************************************************/
package msi.gaml.descriptions;

import static msi.gama.common.interfaces.IGamlIssue.DUPLICATE_DEFINITION;
import static msi.gama.common.interfaces.IGamlIssue.REDEFINES;

import java.util.Collections;
import java.util.Objects;
import java.util.Set;

import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.resource.Resource;

import com.google.common.collect.Iterables;

Expand Down Expand Up @@ -46,21 +51,47 @@ public ExperimentDescription(final String name, final Class<?> clazz, final Spec
super(name, clazz, superDesc, parent, helper, skills2, ff, plugin);
}

private void addParameterNoCheck(final VariableDescription var) {
if (parameters == null) {
parameters = GamaMapFactory.create();
private void addParameter(final VariableDescription var) {
if (parameters == null) { parameters = GamaMapFactory.create(); }
String vName = var.getName();
VariableDescription existing = parameters.get(vName);
if (existing != null) {
existing.warning("'" + vName + "' is overwritten in this experiment and will not be used.",
DUPLICATE_DEFINITION, NAME);
var.warning("'" + vName + "' overwrites a previous definition.", DUPLICATE_DEFINITION, NAME);
}
ModelDescription md = this.getModelDescription();
md.visitAllAttributes(d -> {
VariableDescription vd = (VariableDescription) d;
if (vName.equals(vd.getParameterName())) {
// Possibily different resources
final Resource newResource =
var.getUnderlyingElement() == null ? null : var.getUnderlyingElement().eResource();
final Resource existingResource = vd.getUnderlyingElement().eResource();
if (Objects.equals(newResource, existingResource)) {
var.info("'" + vName + "' supersedes the parameter declaration in " + vd.getOriginName(), REDEFINES,
NAME);
vd.info("Parameter '" + vName + "' is redefined in experiment "
+ var.getEnclosingDescription().getName(), DUPLICATE_DEFINITION, NAME);
} else {
var.info("This definition of '" + vName + "' supersedes the one in imported file "
+ existingResource.getURI().lastSegment(), REDEFINES, NAME);
}
}
return true;
}

);
parameters.put(var.getName(), var);
}

public boolean hasParameter(final String name) {
if (parameters == null) { return false; }
if (parameters == null) return false;
return parameters.containsKey(name);
}

public VariableDescription getParameter(final String name) {
if (parameters == null) { return null; }
if (parameters == null) return null;
return parameters.get(name);
}

Expand All @@ -78,36 +109,32 @@ public void addInheritedParameter(final VariableDescription vd) {

// If no previous definition is found, just add the parameter
if (!hasParameter(inheritedVarName)) {
addParameterNoCheck(vd.copy(this));
addParameter(vd.copy(this));
return;
}
// A redefinition has been found
final VariableDescription existing = getParameter(inheritedVarName);
if (assertAttributesAreCompatible(vd, existing)) {
if (!existing.isBuiltIn()) {
markAttributeRedefinition(vd, existing);
}
if (!existing.isBuiltIn()) { markAttributeRedefinition(vd, existing); }
existing.copyFrom(vd);
}
}

@Override
public void addInheritedAttribute(final VariableDescription var) {
if (var.getKeyword().equals(PARAMETER)) {
if (!hasParameter(var.getName())) {
addParameterNoCheck(var);
}
if (PARAMETER.equals(var.getKeyword())) {
addParameter(var);
} else {
super.addInheritedAttribute(var);
}
}

@Override
public void addOwnAttribute(final VariableDescription var) {
if (!var.getKeyword().equals(PARAMETER)) {
if (!PARAMETER.equals(var.getKeyword())) {
super.addOwnAttribute(var);
} else {
addParameterNoCheck(var);
addParameter(var);
}
}

Expand All @@ -127,29 +154,24 @@ public boolean isExperiment() {

@Override
public boolean visitOwnChildren(final DescriptionVisitor<IDescription> visitor) {
if (!super.visitOwnChildren(visitor)) { return false; }
if (parameters != null) {
if (!parameters.forEachValue(visitor)) { return false; }
}
if (output != null) {
if (!visitor.process(output)) { return false; }
}
if (permanent != null) {
if (!visitor.process(permanent)) { return false; }
}
if (!super.visitOwnChildren(visitor) || parameters != null && !parameters.forEachValue(visitor)) return false;
if (output != null && !visitor.process(output) || permanent != null && !visitor.process(permanent))
return false;
return true;
}

@Override
public boolean visitOwnChildrenRecursively(final DescriptionVisitor<IDescription> visitor) {
final DescriptionVisitor<IDescription> recursiveVisitor = each -> {
if (!visitor.process(each)) { return false; }
if (!visitor.process(each)) return false;
return each.visitOwnChildrenRecursively(visitor);
};
if (!super.visitOwnChildrenRecursively(visitor)) { return false; }
if (parameters != null && !parameters.forEachValue(recursiveVisitor)) { return false; }
if (output != null && !recursiveVisitor.process(output)) { return false; }
if (permanent != null && !recursiveVisitor.process(permanent)) { return false; }
if (!super.visitOwnChildrenRecursively(visitor)
|| parameters != null && !parameters.forEachValue(recursiveVisitor))
return false;
if (output != null && !recursiveVisitor.process(output)
|| permanent != null && !recursiveVisitor.process(permanent))
return false;
return true;
}

Expand All @@ -164,18 +186,12 @@ public Iterable<IDescription> getOwnChildren() {
@Override
public boolean visitChildren(final DescriptionVisitor<IDescription> visitor) {
boolean result = super.visitChildren(visitor);
if (!result) { return false; }
if (parameters != null) {
result &= parameters.forEachValue(visitor);
}
if (!result) { return false; }
if (output != null) {
result &= visitor.process(output);
}
if (!result) { return false; }
if (permanent != null) {
result &= visitor.process(permanent);
}
if (!result) return false;
if (parameters != null) { result &= parameters.forEachValue(visitor); }
if (!result) return false;
if (output != null) { result &= visitor.process(output); }
if (!result) return false;
if (permanent != null) { result &= visitor.process(permanent); }
return result;
}

Expand All @@ -194,13 +210,9 @@ public Boolean isMemorize() {
}

public String getExperimentType() {
if (isBatch()) {
return IKeyword.BATCH;
} else if (isMemorize()) {
return IKeyword.MEMORIZE;
} else {
return IKeyword.GUI_;
}
if (isBatch()) return IKeyword.BATCH;
if (isMemorize()) return IKeyword.MEMORIZE;
return IKeyword.GUI_;
}

@Override
Expand Down Expand Up @@ -251,9 +263,9 @@ private void mergeOutputs(final StatementDescription inherited, final StatementD

@Override
protected void addBehavior(final StatementDescription r) {
if (r.getKeyword().equals(OUTPUT)) {
if (OUTPUT.equals(r.getKeyword())) {
output = r;
} else if (r.getKeyword().equals(PERMANENT)) {
} else if (PERMANENT.equals(r.getKeyword())) {
permanent = r;
} else {
super.addBehavior(r);
Expand All @@ -262,12 +274,10 @@ protected void addBehavior(final StatementDescription r) {

@Override
protected boolean parentIsVisible() {
if (!getParent().isExperiment()) { return false; }
if (parent.isBuiltIn()) { return true; }
if (!getParent().isExperiment()) return false;
if (parent.isBuiltIn()) return true;
final ModelDescription host = (ModelDescription) getMacroSpecies();
if (host != null) {
if (host.getExperiment(parent.getName()) != null) { return true; }
}
if (host != null && host.getExperiment(parent.getName()) != null) return true;
return false;
}

Expand All @@ -276,4 +286,16 @@ public boolean visitMicroSpecies(final DescriptionVisitor<SpeciesDescription> vi
return true;
}

// @Override
// protected boolean validateChildren() {
// // We verify that parameters have different titles and that the model does not declare attributes with duplicate
// // parameter facets
// Map<VariableDescription, String> reverse = GamaMapFactory.create();
// ListMultimap<String, VariableDescription> mm = MultimapBuilder.linkedHashKeys().arrayListValues().build();
// if (parameters != null) {
// parameters.forEach((s, v) -> mm.put(v.getTitle(), v));
// }
// return super.validateChildren();
// }

}
50 changes: 5 additions & 45 deletions msi.gama.core/src/msi/gaml/descriptions/SpeciesDescription.java
View file
Expand Up @@ -572,9 +572,8 @@ public boolean process(final SpeciesDescription desc) {
if (desc == parent) {
result[0] = true;
return false;
} else {
desc.visitMicroSpecies(this);
}
desc.visitMicroSpecies(this);
return true;
}
});
Expand All @@ -586,11 +585,8 @@ private boolean hierarchyContainsSelf() {
while (currentSpeciesDesc != null) {
final SpeciesDescription p = currentSpeciesDesc.getParent();
// Takes care of invalid species (see Issue 711)
if (p == currentSpeciesDesc || p == this)
return true;
else {
currentSpeciesDesc = p;
}
if (p == currentSpeciesDesc || p == this) return true;
currentSpeciesDesc = p;
}
return false;
}
Expand All @@ -599,11 +595,8 @@ protected boolean parentIsVisible() {
if (getParent().isExperiment()) return false;
SpeciesDescription host = getMacroSpecies();
while (host != null) {
if (host == parent || host.getMicroSpecies(parent.getName()) != null)
return true;
else {
host = host.getMacroSpecies();
}
if (host == parent || host.getMicroSpecies(parent.getName()) != null) return true;
host = host.getMacroSpecies();
}
return false;
}
Expand Down Expand Up @@ -719,39 +712,6 @@ public IMap<String, SpeciesDescription> getMicroSpecies() {
return microSpecies;
}

// public Iterable<SpeciesDescription> getSortedMicroSpecies() {
// final GamaTree<SpeciesDescription> tree = GamaTree.withRoot(this);
//
// final Iterable<SpeciesDescription> before = microSpecies.values();
// DEBUG.OUT(Iterables.transform(before, each -> each.getName()));
// final boolean[] found = { false };
// for (final SpeciesDescription sd : before) {
// tree.visitPreOrder(tree.getRoot(), n -> {
// if (sd.getParent() == n.getData()) {
// n.addChild(sd);
// found[0] = true;
// }
// });
// if (found[0]) {
// found[0] = false;
// } else {
// tree.getRoot().addChild(sd);
// }
// }
// DEBUG.OUT(Iterables.transform(tree.list(Order.PRE_ORDER), each -> each.getData().getName()));
// final List<GamaNode<SpeciesDescription>> list = tree.list(Order.PRE_ORDER);
// list.remove(0);
// return Iterables.transform(list, each -> each.getData());
//
// }

// protected void addAttributeNoCheck(final VariableDescription vd) {
// super.addAttributeNoCheck(vd);
// this.getSkills().forEach(s->{
// if (s.)
// });
// }

public boolean isMirror() {
return hasFacet(MIRRORS);
}
Expand Down

0 comments on commit 2af799f

Please sign in to comment.