Skip to content

Commit 7dd4957

Browse files
committed
Coercion of string "false" to boolean now provides a warning.
This is usually not what the user meant to do, as string "false" evaluates to boolean true. Therefore, when we try to coerce a string into a boolean, we first check to see if it is the string "false" and if so, issue a warning. Eventually, I would like this to be configurable per file, or at least at a more granular level, but anyways, for now this is a globally configurable warning in the logger-preferences.ini settings. This also required a change to the Static.getBoolean method, which has been updated throughout the codebase to add a Target parameter. However, because extensions may be using this method, it remains backwards compatible for now, but the no Target method is deprecated and should be removed at next major version change.
1 parent e17b850 commit 7dd4957

40 files changed

+279
-174
lines changed

src/main/java/com/laytonsmith/core/ArgumentValidation.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,11 @@ public static boolean getBoolean(Construct c, Target t) {
337337
if(c instanceof CBoolean) {
338338
b = ((CBoolean) c).getBoolean();
339339
} else if(c instanceof CString) {
340+
if(((CString)c).val().equals("false")) {
341+
CHLog.GetLogger().e(CHLog.Tags.FALSESTRING, "String \"false\" evaluates as true (non-empty strings are"
342+
+ " true). This is most likely not what you meant to do. This warning can globally be disabled"
343+
+ " with the logger-preferences.ini file.", t);
344+
}
340345
b = (c.val().length() > 0);
341346
} else if(c instanceof CInt || c instanceof CDouble) {
342347
b = !(getNumber(c, t) == 0);

src/main/java/com/laytonsmith/core/CHLog.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ private CHLog() {
4747
public enum Tags {
4848
COMPILER("compiler", "Logs compiler errors (but not runtime errors)", LogLevel.WARNING),
4949
RUNTIME("runtime", "Logs runtime errors, (exceptions that bubble all the way to the top)", LogLevel.ERROR),
50+
FALSESTRING("falsestring", "Logs coersion of the string \"false\" to boolean, which is actually true",
51+
LogLevel.ERROR),
5052
DEPRECATION("deprecation", "Shows deprecation warnings", LogLevel.WARNING),
5153
PERSISTENCE("persistence", "Logs when any persistence actions occur.", LogLevel.ERROR),
5254
//TODO Add the rest of these hooks into the code
@@ -115,7 +117,12 @@ private static LogLevel GetLevel(Tags tag) {
115117
}
116118
LogLevel level;
117119
try {
118-
level = LogLevel.valueOf((String) prefs.getPreference(tag.name));
120+
String pref = (String) prefs.getPreference(tag.name);
121+
if("ON".equals(pref)) {
122+
level = LogLevel.ERROR;
123+
} else {
124+
level = LogLevel.valueOf(pref);
125+
}
119126
} catch(IllegalArgumentException e) {
120127
level = LogLevel.ERROR;
121128
}

src/main/java/com/laytonsmith/core/ObjectGenerator.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ public MCItemMeta itemMeta(Construct c, MCMaterial mat, Target t) throws ConfigR
576576

577577
if(Static.getServer().getMinecraftVersion().gte(MCVersion.MC1_11)) {
578578
if(ma.containsKey("unbreakable")) {
579-
meta.setUnbreakable(Static.getBoolean(ma.get("unbreakable", t)));
579+
meta.setUnbreakable(Static.getBoolean(ma.get("unbreakable", t), t));
580580
}
581581
}
582582
}
@@ -1040,10 +1040,10 @@ public List<MCLivingEntity.MCEffect> potions(CArray ea, Target t) {
10401040
}
10411041
}
10421042
if(effect.containsKey("ambient")) {
1043-
ambient = Static.getBoolean(effect.get("ambient", t));
1043+
ambient = Static.getBoolean(effect.get("ambient", t), t);
10441044
}
10451045
if(effect.containsKey("particles")) {
1046-
particles = Static.getBoolean(effect.get("particles", t));
1046+
particles = Static.getBoolean(effect.get("particles", t), t);
10471047
}
10481048
ret.add(new MCLivingEntity.MCEffect(potionID, strength, (int) (seconds * 20), ambient, particles));
10491049
} else {
@@ -1116,10 +1116,10 @@ public CArray fireworkEffect(MCFireworkEffect mcfe, Target t) {
11161116
public MCFireworkEffect fireworkEffect(CArray fe, Target t) {
11171117
MCFireworkBuilder builder = StaticLayer.GetConvertor().GetFireworkBuilder();
11181118
if(fe.containsKey("flicker")) {
1119-
builder.setFlicker(Static.getBoolean(fe.get("flicker", t)));
1119+
builder.setFlicker(Static.getBoolean(fe.get("flicker", t), t));
11201120
}
11211121
if(fe.containsKey("trail")) {
1122-
builder.setTrail(Static.getBoolean(fe.get("trail", t)));
1122+
builder.setTrail(Static.getBoolean(fe.get("trail", t), t));
11231123
}
11241124
if(fe.containsKey("colors")) {
11251125
Construct colors = fe.get("colors", t);

src/main/java/com/laytonsmith/core/Static.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,28 @@ public static byte getInt8(Construct c, Target t) {
238238
* is empty, it is false, otherwise it is true.
239239
*
240240
* @param c
241+
* @param t
241242
* @return
242243
*/
244+
public static boolean getBoolean(Construct c, Target t) {
245+
return ArgumentValidation.getBoolean(c, t);
246+
}
247+
248+
/**
249+
* Returns a boolean from any given construct. Depending on the type of the construct being converted, it follows
250+
* the following rules: If it is an integer or a double, it is false if 0, true otherwise. If it is a string, if it
251+
* is empty, it is false, otherwise it is true.
252+
*
253+
* @param c
254+
* @return
255+
* @deprecated Use
256+
* {@link #getBoolean(com.laytonsmith.core.constructs.Construct, com.laytonsmith.core.constructs.Target)}
257+
* instead, as it provides better error messages for users that use the string "false" as a boolean. This method
258+
* should be removed in version 3.3.3 or above.
259+
*/
260+
@Deprecated
243261
public static boolean getBoolean(Construct c) {
244-
return ArgumentValidation.getBoolean(c, Target.UNKNOWN);
262+
return getBoolean(c, Target.UNKNOWN);
245263
}
246264

247265
/**

src/main/java/com/laytonsmith/core/compiler/FileOptions.java

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
package com.laytonsmith.core.compiler;
22

3+
import com.laytonsmith.PureUtilities.ClassLoading.ClassDiscovery;
4+
import com.laytonsmith.PureUtilities.Version;
5+
import com.laytonsmith.core.CHVersion;
6+
import com.laytonsmith.core.Documentation;
37
import com.laytonsmith.core.Prefs;
8+
import java.net.URL;
49
import java.util.ArrayList;
10+
import java.util.EnumSet;
511
import java.util.List;
612
import java.util.Map;
13+
import java.util.Set;
714

815
/**
916
*
@@ -12,10 +19,10 @@
1219
public class FileOptions {
1320

1421
/*
15-
These values are used in the syntax highlighter, and should remain the name they are in code.
22+
These value names are used in the syntax highlighter, and should remain the name they are in code.
1623
*/
1724
private final Boolean strict;
18-
private final List<String> suppressWarnings;
25+
private final Set<SuppressWarnings> suppressWarnings;
1926
private final String name;
2027
private final String author;
2128
private final String created;
@@ -24,7 +31,7 @@ public class FileOptions {
2431

2532
public FileOptions(Map<String, String> parsedOptions) {
2633
strict = parseBoolean(getDefault(parsedOptions, "strict", null));
27-
suppressWarnings = parseList(getDefault(parsedOptions, "suppresswarnings", ""));
34+
suppressWarnings = parseEnumSet(getDefault(parsedOptions, "suppresswarnings", ""), SuppressWarnings.class);
2835
name = getDefault(parsedOptions, "name", "");
2936
author = getDefault(parsedOptions, "author", "");
3037
created = getDefault(parsedOptions, "created", "");
@@ -55,6 +62,20 @@ private List<String> parseList(String list) {
5562
}
5663
return l;
5764
}
65+
66+
private <T extends Enum<T>> Set<T> parseEnumSet(String list, Class<T> type) {
67+
EnumSet<T> set = EnumSet.noneOf(type);
68+
List<String> sList = parseList(list);
69+
for(String s : sList) {
70+
for(T e : type.getEnumConstants()) {
71+
if(e.name().equals(s)) {
72+
set.add(e);
73+
break;
74+
}
75+
}
76+
}
77+
return set;
78+
}
5879

5980
/**
6081
* Returns whether or not this file is in strict mode. Unlike most options, this one depends on both the file
@@ -71,8 +92,8 @@ public boolean isStrict() {
7192
}
7293
}
7394

74-
public boolean isWarningSuppressed(String warning) {
75-
return warning.trim().contains(warning.toLowerCase());
95+
public boolean isWarningSuppressed(SuppressWarnings warning) {
96+
return suppressWarnings.contains(warning);
7697
}
7798

7899
public String getName() {
@@ -101,5 +122,45 @@ public String toString() {
101122
+ (description == null ? "" : "File description: " + description + "\n");
102123

103124
}
125+
126+
public static enum SuppressWarnings implements Documentation {
127+
// In the future, when some are added, this can be removed, and the rest of the system will work
128+
// quite nicely. Perhaps a good first candidate would be to allow string "false" coerced to boolean warning
129+
// to be suppressed on a per file basis?
130+
Note("There are currently no warning suppressions defined, but some will be added in the future",
131+
CHVersion.V0_0_0);
132+
133+
private SuppressWarnings(String docs, Version version) {
134+
this.docs = docs;
135+
this.version = version;
136+
}
137+
138+
private final String docs;
139+
private final Version version;
140+
@Override
141+
public URL getSourceJar() {
142+
return ClassDiscovery.GetClassContainer(this.getClass());
143+
}
144+
145+
@Override
146+
public Class<? extends Documentation>[] seeAlso() {
147+
return new Class[]{};
148+
}
149+
150+
@Override
151+
public String getName() {
152+
return this.name();
153+
}
154+
155+
@Override
156+
public String docs() {
157+
return docs;
158+
}
159+
160+
@Override
161+
public Version since() {
162+
return version;
163+
}
164+
}
104165

105166
}

src/main/java/com/laytonsmith/core/constructs/CArray.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -876,11 +876,11 @@ public int compare(Construct o1, Construct o2) {
876876
}
877877
}
878878
if(o1 instanceof CBoolean || o2 instanceof CBoolean) {
879-
if(Static.getBoolean(o1) == Static.getBoolean(o2)) {
879+
if(Static.getBoolean(o1, Target.UNKNOWN) == Static.getBoolean(o2, Target.UNKNOWN)) {
880880
return 0;
881881
} else {
882-
int oo1 = Static.getBoolean(o1) ? 1 : 0;
883-
int oo2 = Static.getBoolean(o2) ? 1 : 0;
882+
int oo1 = Static.getBoolean(o1, Target.UNKNOWN) ? 1 : 0;
883+
int oo2 = Static.getBoolean(o2, Target.UNKNOWN) ? 1 : 0;
884884
return (oo1 < oo2) ? -1 : 1;
885885
}
886886
}
@@ -899,13 +899,13 @@ public int compare(Construct o1, Construct o2) {
899899
}
900900

901901
public int compareRegular(Construct o1, Construct o2) {
902-
if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o1))
903-
&& Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o2))) {
902+
if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o1), Target.UNKNOWN)
903+
&& Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o2), Target.UNKNOWN)) {
904904
return compareNumeric(o1, o2);
905-
} else if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o1))) {
905+
} else if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o1), Target.UNKNOWN)) {
906906
//The first is a number, the second is a string
907907
return -1;
908-
} else if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o2))) {
908+
} else if(Static.getBoolean(new DataHandling.is_numeric().exec(Target.UNKNOWN, null, o2), Target.UNKNOWN)) {
909909
//The second is a number, the first is a string
910910
return 1;
911911
} else {

src/main/java/com/laytonsmith/core/events/Prefilters.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ private static void ItemMatch(Construct item1, Construct item2) throws Prefilter
152152
}
153153

154154
private static void BooleanMatch(Construct bool1, Construct bool2) throws PrefilterNonMatchException {
155-
if(Static.getBoolean(bool1) != Static.getBoolean(bool2)) {
155+
if(Static.getBoolean(bool1, Target.UNKNOWN) != Static.getBoolean(bool2, Target.UNKNOWN)) {
156156
throw new PrefilterNonMatchException();
157157
}
158158
}

src/main/java/com/laytonsmith/core/events/drivers/CmdlineEvents.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public boolean matches(Map<String, Construct> prefilter, BindableEvent e) throws
155155
@Override
156156
public BindableEvent convert(CArray manualObject, Target t) {
157157
CmdlinePromptInput cpi = new CmdlinePromptInput(manualObject.get("command", t).val(),
158-
Static.getBoolean(manualObject.get("shellMode", t)));
158+
Static.getBoolean(manualObject.get("shellMode", t), t));
159159
return cpi;
160160
}
161161

src/main/java/com/laytonsmith/core/events/drivers/PlayerEvents.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,10 +1300,10 @@ public boolean modifyEvent(String key, Construct value, BindableEvent event) {
13001300
e.setDeathMessage(value.nval());
13011301
return true;
13021302
case "keep_inventory":
1303-
e.setKeepInventory(Static.getBoolean(value));
1303+
e.setKeepInventory(Static.getBoolean(value, Target.UNKNOWN));
13041304
return true;
13051305
case "keep_level":
1306-
e.setKeepLevel(Static.getBoolean(value));
1306+
e.setKeepLevel(Static.getBoolean(value, Target.UNKNOWN));
13071307
return true;
13081308
case "new_exp":
13091309
e.setNewExp(Static.getInt32(value, Target.UNKNOWN));
@@ -2447,7 +2447,7 @@ public boolean modifyEvent(String key, Construct value, BindableEvent event) {
24472447
return true;
24482448
}
24492449
} else if(key.equalsIgnoreCase("signing")) {
2450-
((MCPlayerEditBookEvent) event).setSigning(Static.getBoolean(value));
2450+
((MCPlayerEditBookEvent) event).setSigning(Static.getBoolean(value, Target.UNKNOWN));
24512451
return true;
24522452
} else {
24532453
return false;

src/main/java/com/laytonsmith/core/events/drivers/VehicleEvents.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,11 @@ public boolean modifyEvent(String key, Construct value, BindableEvent event) {
301301
if(event instanceof MCVehicleEnitityCollideEvent) {
302302
MCVehicleEnitityCollideEvent e = (MCVehicleEnitityCollideEvent) event;
303303
if(key.equals("collide")) {
304-
e.setCollisionCancelled(!Static.getBoolean(value));
304+
e.setCollisionCancelled(!Static.getBoolean(value, Target.UNKNOWN));
305305
return true;
306306
}
307307
if(key.equals("pickup")) {
308-
e.setPickupCancelled(!Static.getBoolean(value));
308+
e.setPickupCancelled(!Static.getBoolean(value, Target.UNKNOWN));
309309
return true;
310310
}
311311
}

0 commit comments

Comments
 (0)