From bb70113d0f889a2c862e6f6436db22901f545b81 Mon Sep 17 00:00:00 2001
From: spookydonut <github@spooksoftware.com>
Date: Sun, 19 Jan 2020 06:22:07 +0800
Subject: [PATCH] Fix regression on defined() preprocessing (#152)

Fixes #96.

0a650be853b005ab4a333c97ff5e9372fccd237c changed the preprocessor to
always substitute macros for their value if they were defined, as such
`#if defined(FOO)` would cause a lint error if `FOO` was defined.

There were two different errors with the exact same message, the first
would happen if `#define FOO` because `defined()` would have no
arguments.  The second would happen if `#define FOO 1` because `1`
isn't an ident.

Because of those tangentially related things I discovered in my
attempts to fix the bug, I've changed the error messages to be more
specific.
---
 src/dreammaker/constants.rs    | 10 +++++-----
 src/dreammaker/preprocessor.rs | 14 +++++++++++++-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/dreammaker/constants.rs b/src/dreammaker/constants.rs
index 61ba01d8..18f7c2bd 100644
--- a/src/dreammaker/constants.rs
+++ b/src/dreammaker/constants.rs
@@ -743,7 +743,7 @@ impl<'a> ConstantFolder<'a> {
                 "rgb" => {
                     use std::fmt::Write;
                     if args.len() != 3 && args.len() != 4 {
-                        return Err(self.error("malformed rgb() call"));
+                        return Err(self.error(format!("malformed rgb() call, must have 3 or 4 arguments and instead has {}", args.len())));
                     }
                     let mut result = String::with_capacity(7);
                     result.push_str("#");
@@ -752,7 +752,7 @@ impl<'a> ConstantFolder<'a> {
                             let clamped = std::cmp::max(::std::cmp::min(i, 255), 0);
                             let _ = write!(result, "{:02x}", clamped);
                         } else {
-                            return Err(self.error("malformed rgb() call"));
+                            return Err(self.error("malformed rgb() call, argument wasn't an int"));
                         }
                     }
                     Constant::String(result)
@@ -760,7 +760,7 @@ impl<'a> ConstantFolder<'a> {
                 "defined" if self.defines.is_some() => {
                     let defines = self.defines.unwrap();  // annoying, but keeps the match clean
                     if args.len() != 1 {
-                        return Err(self.error("malformed defined() call"));
+                        return Err(self.error(format!("malformed defined() call, must have 1 argument and instead has {}", args.len())));
                     }
                     match args[0] {
                         Expression::Base {
@@ -770,7 +770,7 @@ impl<'a> ConstantFolder<'a> {
                         } if unary.is_empty() && follow.is_empty() => {
                             Constant::Int(if defines.contains_key(ident) { 1 } else { 0 })
                         },
-                        _ => return Err(self.error("malformed defined() call")),
+                        _ => return Err(self.error("malformed defined() call, argument given isn't an Ident.")),
                     }
                 }
                 // other functions are no-goes
@@ -789,7 +789,7 @@ impl<'a> ConstantFolder<'a> {
 
     fn trig_op(&mut self, mut args: Vec<Expression>, op: fn(f32) -> f32) -> Result<Constant, DMError> {
         if args.len() != 1 {
-            Err(self.error("trig function requires exactly 1 argument"))
+            Err(self.error(format!("trig function requires exactly 1 argument, instead found {}", args.len())))
         } else if let Some(f) = self.expr(args.remove(0), None)?.to_float() {
             Ok(Constant::Float(op(f)))
         } else {
diff --git a/src/dreammaker/preprocessor.rs b/src/dreammaker/preprocessor.rs
index b51bfddb..b7ca91c9 100644
--- a/src/dreammaker/preprocessor.rs
+++ b/src/dreammaker/preprocessor.rs
@@ -930,7 +930,7 @@ impl<'ctx> Preprocessor<'ctx> {
             // anything other than directives may be ifdef'd out
             _ if disabled => return Ok(()),
             // identifiers may be macros
-            Token::Ident(ref ident, _) => {
+            Token::Ident(ref ident, whitespace) => {
                 self.flush_docs();
 
                 // lint for BYOND bug
@@ -955,6 +955,18 @@ impl<'ctx> Preprocessor<'ctx> {
                     return Ok(());
                 }
 
+                // special case for inside a defined() call
+                if let Some(Token::Punct(Punctuation::LParen)) = self.output.back() {
+                    if let Some(idx) = self.output.len().checked_sub(2) {
+                        if let Some(Token::Ident(identname, _)) = self.output.get(idx) {
+                            if identname.as_str() == "defined" {
+                                self.output.push_back(Token::Ident(ident.to_owned(), whitespace));
+                                return Ok(());
+                            }
+                        }
+                    }
+                }
+
                 // if it's a define, perform the substitution
                 let mut expansion = self.defines.get(ident).cloned();  // TODO: don't clone?
                 if expansion.is_some() && self.include_stack.stack.len() > MAX_RECURSION_DEPTH {