Warn on weird values given to built-in set directives (#168)

SpaceManiac · Mar 3, 2020 · d9386bd · d9386bd
1 parent 6ca656a
commit d9386bd
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 5 deletions.
diff --git a/CONFIGURING.md b/CONFIGURING.md
@@ -26,6 +26,7 @@ Raised by DreamChecker:
 * `disabled_directive` - Raised when attempting to disable a `set SpacemanDMM_*` directive that cannot be disabled
 * `sets_directive_twice` - Raised when a directive is set twice in the same proc
 * `invalid_lint_directive_value` - Raised when attempting to set a directive value to something other than `1`, `0`, `TRUE`, `FALSE`
+* `invalid_set_value` - Raised on invalid values used with builtin set directives
 * `unknown_linter_setting` - Raised when setting a `SpacemanDMM_*` directive that DreamChecker doesn't implement
 * `override_missing_keyword_arg` - Raised when proc overrides are missing keyword arguments
 * `must_not_override` - `SpacemanDMM_should_not_override` directive

diff --git a/src/dreamchecker/lib.rs b/src/dreamchecker/lib.rs
@@ -416,8 +416,7 @@ pub fn directive_value_to_truthy(expr: &Expression, location: Location) -> Resul
         Some(Term::Int(1)) => Ok(true),
         Some(Term::Ident(i)) if i == "FALSE" => Ok(false),
         Some(Term::Ident(i)) if i == "TRUE" => Ok(true),
-        _ => Err(error(location, format!("invalid value for lint directive {:?}", expr))
-        .with_errortype("invalid_lint_directive_value")
+        _ => Err(error(location, format!("invalid value for set {:?}", expr))
         .set_severity(Severity::Warning)),
     }
 }
@@ -473,7 +472,7 @@ impl<'o> AnalyzeObjectTree<'o> {
                     self.context.register_error(error);
                 }
             },
-            Err(error) => self.context.register_error(error),
+            Err(error) => self.context.register_error(error.with_errortype("invalid_lint_directive_value")),
         }
     }
 
@@ -500,6 +499,45 @@ impl<'o> AnalyzeObjectTree<'o> {
                     error(statement.location, format!("unknown setting {:?}", name))
                         .set_severity(Severity::Warning)
                         .register(self.context);
+                } else {
+                    match name.as_str() {
+                        "background" | "waitfor" | "hidden" | "instant" | "popup_menu" => {
+                            if let Err(_) = directive_value_to_truthy(value, statement.location) {
+                                error(statement.location, format!("set {} must be 0/1/TRUE/FALSE", name.as_str()))
+                                    .set_severity(Severity::Warning)
+                                    .with_errortype("invalid_set_value")
+                                    .register(self.context);
+                            }
+                        },
+                        "name" | "category" | "desc" => {
+                            if let Some(term) = value.as_term() {
+                                match term {
+                                    // TODO: detect procs-as-verbs here
+                                    Term::String(_) | Term::InterpString(_, _) => {},
+                                    // category can be set null to hide it
+                                    Term::Null if name.as_str() == "category" => {},
+                                    other => {
+                                        error(statement.location, format!("set {} must have a string value", name.as_str()))
+                                            .set_severity(Severity::Warning)
+                                            .with_errortype("invalid_set_value")
+                                            .register(self.context);
+                                    },
+                                }
+                            }
+                        },
+                        "invisibility" => {
+                            if let Some(Term::Int(i)) = value.as_term() {
+                                if *i >= 0 && *i <= 100 {
+                                    continue;
+                                }
+                            }
+                            error(statement.location, "set invisibility must be 0-100")
+                                .set_severity(Severity::Warning)
+                                .with_errortype("invalid_set_value")
+                                .register(self.context);
+                        },
+                        _ => {},
+                    }
                 }
             } else {
                 break;

diff --git a/src/dreammaker/ast.rs b/src/dreammaker/ast.rs
@@ -895,7 +895,6 @@ pub const KNOWN_SETTING_NAMES: &[&str] = &[
     "invisibility",
     "src",
     "background",
-    // undocumented
     "waitfor",
 ];
 

diff --git a/src/dreammaker/parser.rs b/src/dreammaker/parser.rs
@@ -1350,7 +1350,6 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
             };
             let value = require!(self.expression());
             require!(self.statement_terminator());
-            // TODO: warn on weird values for these
             spanned(Statement::Setting { name, mode, value })
         } else if let Some(()) = self.exact_ident("break")? {
             let label = self.ident()?;