provide error for define comment #472

bin/libs/error/src/lib.rs

@@ -78,7 +78,11 @@ impl PrettyError for Error {
     }
 
     fn details(&self) -> Option<String> {
-        None
+        match self {
+            Self::Preprocessor(e) => e.details(),
+            Self::Config(e) => e.details(),
+            _ => None,
+        }
     }
 
     fn help(&self) -> Option<String> {

libs/config/src/model/array.rs

@@ -48,7 +48,8 @@ impl Parse for Array {
         let mut elements = Vec::new();
         let mut first = true;
         loop {
-            let last = whitespace::skip_newline(tokens);
+            let skipped = whitespace::skip_newline(tokens);
+            let last = skipped.last().cloned();
             if let Some(token) = tokens.peek() {
                 if token.symbol() == &Symbol::RightBrace {
                     if first || options.array_allow_trailing_comma() {
@@ -68,7 +69,8 @@ impl Parse for Array {
             let entry = Entry::parse(options, tokens, from)?;
             elements.push(entry);
             first = false;
-            let last = whitespace::skip_newline(tokens);
+            let skipped = whitespace::skip_newline(tokens);
+            let last = skipped.last().cloned();
             if let Some(token) = tokens.next() {
                 if token.symbol() == &Symbol::RightBrace {
                     break;

libs/config/src/model/class.rs

@@ -91,12 +91,18 @@ impl Parse for Class {
         whitespace::skip(tokens);
         let name = Ident::parse(options, tokens, from)?;
         // Check for : and parent
-        let last = whitespace::skip(tokens);
+        let skipped = whitespace::skip(tokens);
+        let last = skipped.last().cloned();
         let parent = if let Some(token) = tokens.peek() {
             if token.symbol() == &Symbol::Colon {
                 tokens.next();
-                let last = whitespace::skip(tokens).unwrap_or_else(|| from.clone());
-                Some(Ident::parse(options, tokens, &last)?)
+                let skipped = whitespace::skip(tokens);
+                let last = skipped.last().cloned();
+                Some(Ident::parse(
+                    options,
+                    tokens,
+                    &last.unwrap_or_else(|| from.clone()),
+                )?)
             } else {
                 None
             }
@@ -106,7 +112,8 @@ impl Parse for Class {
             });
         };
         // read children
-        let last = whitespace::skip_newline(tokens);
+        let skipped = whitespace::skip_newline(tokens);
+        let last = skipped.last().cloned();
         if let Some(token) = tokens.peek() {
             if token.symbol() == &Symbol::Semicolon {
                 return Ok(Self::External { name });
@@ -332,7 +339,8 @@ impl Parse for Properties {
                     }
                     Some(ident) => {
                         if ident.to_string() == "delete" {
-                            let last = whitespace::skip(tokens);
+                            let skipped = whitespace::skip(tokens);
+                            let last = skipped.last().cloned();
                             let ident = Ident::parse(
                                 options,
                                 tokens,
@@ -368,7 +376,8 @@ impl Parse for Properties {
                             });
                         }
                         tokens.next();
-                        let last = whitespace::skip(tokens);
+                        let skipped = whitespace::skip(tokens);
+                        let last = skipped.last().cloned();
                         let entry = if array_expand {
                             let array = Array::parse(options, tokens, from)?;
                             Entry::Array(Array {

libs/config/src/model/entry.rs

@@ -37,7 +37,8 @@ impl Parse for Entry {
     where
         Self: Sized,
     {
-        let last = whitespace::skip_newline(tokens);
+        let skipped = whitespace::skip_newline(tokens);
+        let last = skipped.last().cloned();
         if let Some(token) = tokens.peek() {
             match token.symbol() {
                 Symbol::LeftBrace => {

libs/error/src/lib.rs

@@ -40,8 +40,15 @@ impl AppError {
                 DisplayStyle::Error => format!("{}: ", "error".bright_red()).bold(),
             },
             self.brief.bold(),
-            self.details.clone().unwrap_or_default(),
             self.source().unwrap_or_default(),
+            {
+                let details = self.details.clone().unwrap_or_default();
+                if details.is_empty() {
+                    String::new()
+                } else {
+                    format!("{}: {details}\n", "details".bright_blue())
+                }
+            },
             {
                 let help = self.help.clone().unwrap_or_default();
                 if help.is_empty() {
@@ -190,17 +197,21 @@ pub fn read_lines_from_file(
 /// if the file cannot be read
 pub fn make_source(token: &Token, note: String) -> Result<Source, std::io::Error> {
     Ok(Source {
-        lines: read_lines_from_file(
-            Path::new(
-                token
-                    .source()
-                    .path()
-                    .replace('\\', "/")
-                    .trim_start_matches('/'),
-            ),
-            token.source().start().1 .0,
-            token.source().end().1 .0,
-        )?,
+        lines: if token.source().path().starts_with('%') {
+            Vec::new()
+        } else {
+            read_lines_from_file(
+                Path::new(
+                    token
+                        .source()
+                        .path()
+                        .replace('\\', "/")
+                        .trim_start_matches('/'),
+                ),
+                token.source().start().1 .0,
+                token.source().end().1 .0,
+            )?
+        },
         position: token.source().clone(),
         note,
     })

libs/preprocessor/src/context.rs

@@ -269,7 +269,8 @@ pub enum Definition {
     /// A value that is a list of [`Token`]s to be added at the call site
     Value(Vec<Token>),
     /// A flag that can be checked with `#ifdef`
-    Unit,
+    /// Tokens are only used for error reporting
+    Unit(Vec<Token>),
 }
 
 impl Definition {
@@ -288,7 +289,7 @@ impl Definition {
     #[must_use]
     /// Check if the definition is a flag
     pub const fn is_unit(&self) -> bool {
-        matches!(self, Self::Unit)
+        matches!(self, Self::Unit(_))
     }
 }
 

libs/preprocessor/src/error.rs

@@ -98,6 +98,8 @@ pub enum Error {
         token: Box<Token>,
         /// The [`Token`] stack trace
         trace: Vec<Token>,
+        /// Skipped tokens of Unit
+        skipped: Vec<Token>,
     },
     #[error("`#include` was encountered while using `NoResolver`")]
     /// Tried to use `#include` with [`NoResolver`](crate::resolver::resolvers::NoResolver)
@@ -197,7 +199,7 @@ impl PrettyError for Error {
             } => {
                 format!("Function call with incorrect number of arguments, expected `{expected}` got `{got}`. `{symbol:?}`", symbol = token.symbol())
             }
-            Self::ExpectedFunctionOrValue { token, trace: _ } => {
+            Self::ExpectedFunctionOrValue { token, .. } => {
                 format!(
                     "Expected Function or Value, found Unit, `{symbol:?}`",
                     symbol = token.symbol()
@@ -223,11 +225,38 @@ impl PrettyError for Error {
     }
 
     fn details(&self) -> Option<String> {
-        None
+        match self {
+            Self::ExpectedFunctionOrValue { skipped, .. } => {
+                let empty_comment = skipped.iter().all(|t| {
+                    matches!(t.symbol(), Symbol::Comment(_))
+                        || matches!(t.symbol(), Symbol::Whitespace(_))
+                });
+                if empty_comment {
+                    Some(String::from("`#define` with only a comment is considered a Unit (flag). This differs from other preprocessors."))
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        }
     }
 
     fn help(&self) -> Option<String> {
         match self {
+            Self::ExpectedFunctionOrValue { token, skipped, .. } => {
+                let empty_comment = skipped.iter().all(|t| {
+                    matches!(t.symbol(), Symbol::Comment(_))
+                        || matches!(t.symbol(), Symbol::Whitespace(_))
+                });
+                if empty_comment {
+                    Some(format!(
+                        "Try using `#define {} ; /* .. */`",
+                        token.symbol().output()
+                    ))
+                } else {
+                    None
+                }
+            }
             Self::FunctionCallArgumentCount {
                 token,
                 expected: _,
@@ -290,7 +319,7 @@ impl PrettyError for Error {
             } => make_source(token, format!("Expects {expected} arguments"))
                 .ok()
                 .map(Box::new),
-            Self::ExpectedFunctionOrValue { token, trace: _ } => {
+            Self::ExpectedFunctionOrValue { token, .. } => {
                 make_source(token, "expects function or value".to_string())
                     .ok()
                     .map(Box::new)

libs/preprocessor/src/lib.rs

@@ -341,17 +341,15 @@ where
             token: Box::new(from),
         });
     };
-    let mut skipped = false;
-    let mut last = None;
+    let mut skipped = Vec::new();
     if let Some(token) = tokenstream.peek() {
         if let Symbol::Whitespace(_) | Symbol::Comment(_) = token.symbol() {
-            last = whitespace::skip(tokenstream);
-            skipped = true;
+            skipped = whitespace::skip(tokenstream);
         }
     }
     // check directive type
     if let Some(token) = tokenstream.peek() {
-        match (token.symbol(), skipped) {
+        match (token.symbol(), !skipped.is_empty()) {
             (Symbol::LeftParenthesis, false) => {
                 let token = token.clone();
                 let args = read_args(resolver, context, tokenstream, &token, false)?;
@@ -371,23 +369,15 @@ where
                 context.define(ident, ident_token, Definition::Function(def))?;
             }
             (Symbol::Newline, _) => {
-                context.define(ident, ident_token, Definition::Unit)?;
+                context.define(ident, ident_token, Definition::Unit(skipped))?;
             }
             (_, _) => {
                 let val = directive_define_read_body(tokenstream);
                 context.define(ident, ident_token, Definition::Value(val))?;
-                // return Err(Error::UnexpectedToken {
-                //     token: Box::new(token.clone()),
-                //     expected: vec![
-                //         Symbol::LeftParenthesis,
-                //         Symbol::Whitespace(Whitespace::Space),
-                //         Symbol::Whitespace(Whitespace::Tab),
-                //         Symbol::Escape,
-                //     ],
-                // });
             }
         }
     } else {
+        let last = skipped.last().cloned();
         return Err(Error::UnexpectedEOF {
             token: Box::new(last.unwrap_or_else(|| from.clone())),
         });
@@ -614,7 +604,7 @@ where
                     continue;
                 }
                 if recursive {
-                    if let Some((_source, definition)) = context.get(word, token) {
+                    if let Some((source, definition)) = context.get(word, token) {
                         let token = token.clone();
                         tokenstream.next();
                         if definition.is_function()
@@ -623,13 +613,15 @@ where
                             arg.push(tokenstream.next().unwrap());
                             continue;
                         }
+                        context.push(source);
                         arg.append(&mut walk_definition(
                             resolver,
                             context,
                             tokenstream,
                             token,
                             definition,
                         )?);
+                        context.pop();
                         continue;
                     }
                 }
@@ -673,8 +665,9 @@ where
         }
         match token.symbol() {
             Symbol::Word(word) => {
-                if let Some((_source, definition)) = context.get(word, token) {
+                if let Some((source, definition)) = context.get(word, token) {
                     let token = token.clone();
+                    context.push(source);
                     tokenstream.next();
                     output.append(&mut walk_definition(
                         resolver,
@@ -683,6 +676,7 @@ where
                         token,
                         definition,
                     )?);
+                    context.pop();
                 } else {
                     output.push(tokenstream.next().unwrap());
                 }
@@ -798,10 +792,11 @@ where
                 )?);
             }
         }
-        Definition::Unit => {
+        Definition::Unit(skipped) => {
             return Err(Error::ExpectedFunctionOrValue {
                 token: Box::new(from),
                 trace: context.trace(),
+                skipped,
             });
         }
     }
@@ -813,7 +808,7 @@ fn eat_newline(
     context: &mut Context,
     from: &Token,
 ) -> Result<(), Error> {
-    let last = whitespace::skip(tokenstream);
+    let skipped = whitespace::skip(tokenstream);
     if let Some(token) = tokenstream.peek() {
         if matches!(token.symbol(), Symbol::Newline) {
             tokenstream.next();
@@ -825,6 +820,7 @@ fn eat_newline(
             });
         }
     } else {
+        let last = skipped.last().cloned();
         return Err(Error::UnexpectedEOF {
             token: Box::new(last.unwrap_or_else(|| from.clone())),
         });