diff --git a/CHANGELOG.md b/CHANGELOG.md index 008492c..274492e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.2.2] - 2026-02-14 +## [0.2.2] - 2026-02-15 ### Added @@ -13,16 +13,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Description on how to build the LSP framework to correctly compile the Server using the scons - Detailed documentation for some builtin functions (not all yet) -- Missing group_uniforms support (including nested structures) + +- Sematics + - Missing group_uniforms support (including nested structures) + - Detection for recursion calls and appropiate errors ### Changed -- In code documentation +- Symbol Table + - Symbol lookups have a scope depth paremeter now + - Added functionality to lookup all symbols across all scopes + - Symbols now seperate symbol type and mutability. This resolves a lot of problems with tracking builtins and godots "in" or "out" parameters ### Fixed -- Included sky shaders in shader scope -- Sky shaders now correctly have acces to their builtin variables +- Shader type behaviour: + - Included sky shaders in shader scope + - Sky shaders now correctly have acces to their builtin variables + - Default behaviour of shader files that are missing the shader_type declaration no longer is spatial shader + +- Parser + - Const declarations in parseStatement are correctly detected now + +- Semantic Analyzer + - Const evaluation of builtins and user declared symbols + - Binary operations with bools now correctly report an error + - Scalar constructor checks are not skippe anymoren through logic error + - Segfaulting on RootSymbol acces for swizzles and array due to unchecked nullptr return ## [0.2.1] - 2025-12-01 diff --git a/README.md b/README.md index 755d954..095dd7d 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ This project uses **SCons** as its build system. This project expects the **LSP Framework** to be located at `extern/lsp-framework`. You must compile the framework for your target platform and ensure the build artifacts are located in the specific directories expected by the `SConstruct` script: -* **Linux**: `extern/lsp-framework/build` +* **Linux**: `extern/lsp-framework/build_linux` * **Windows**: `extern/lsp-framework/build_windows` * **macOS**: `extern/lsp-framework/build_macos` diff --git a/src/gdshader/builtins.hpp b/src/gdshader/builtins.hpp index f7450de..5303c47 100644 --- a/src/gdshader/builtins.hpp +++ b/src/gdshader/builtins.hpp @@ -12,7 +12,8 @@ namespace gdshader_lsp { // ENUMS // ------------------------------------------------------------------------- -enum class ShaderType { +enum class ShaderType +{ Spatial, CanvasItem, Particles, @@ -21,7 +22,8 @@ enum class ShaderType { Unknown }; -enum class ShaderStage { +enum class ShaderStage +{ Global, Vertex, Fragment, @@ -38,8 +40,9 @@ enum class ShaderStage { struct BuiltinVariable { std::string name; - std::string type; // e.g., "vec3", "float" + std::string type; std::string doc; + std::string qualifier = "in"; }; struct BuiltinFunction { @@ -62,6 +65,7 @@ using BuiltinFuncList = std::vector; static const BuiltinList SPATIAL_VERTEX = { // Global + {"TIME", "float", "Global time in seconds."}, {"PI", "float", "PI constant (3.141592)."}, {"TAU", "float", "TAU constant (6.283185)."}, @@ -90,33 +94,35 @@ static const BuiltinList SPATIAL_VERTEX = { {"VERTEX", "vec3", "Vertex position in model space (or world if using world_vertex_coords)."}, {"VERTEX_ID", "int", "Index of the current vertex."}, - {"NORMAL", "vec3", "Normal in model space."}, - {"TANGENT", "vec3", "Tangent in model space."}, - {"BINORMAL", "vec3", "Binormal in model space."}, - {"POSITION", "vec4", "Override final vertex position in clip space."}, - {"UV", "vec2", "UV main channel."}, - {"UV2", "vec2", "UV secondary channel."}, - {"COLOR", "vec4", "Vertex color."}, + {"NORMAL", "vec3", "Normal in model space.", "inout"}, + {"TANGENT", "vec3", "Tangent in model space.", "inout"}, + {"BINORMAL", "vec3", "Binormal in model space.", "inout"}, + {"POSITION", "vec4", "Override final vertex position in clip space.", "out"}, + {"UV", "vec2", "UV main channel.", "inout"}, + {"UV2", "vec2", "UV secondary channel.", "inout"}, + {"COLOR", "vec4", "Vertex color.", "inout"}, {"ROUGHNESS", "float", "Roughness for vertex lighting."}, - {"POINT_SIZE", "float", "Point size for point rendering."}, + {"POINT_SIZE", "float", "Point size for point rendering.", "inout"}, - {"MODELVIEW_MATRIX", "mat4", "Model space to view space transform."}, - {"MODELVIEW_NORMAL_MATRIX", "mat3", "Model space to view space normal transform."}, + {"MODELVIEW_MATRIX", "mat4", "Model space to view space transform.", "inout"}, + {"MODELVIEW_NORMAL_MATRIX", "mat3", "Model space to view space normal transform.", "inout"}, {"MODEL_MATRIX", "mat4", "Model space to world space transform."}, {"MODEL_NORMAL_MATRIX", "mat3", "Model space to world space normal transform."}, - {"PROJECTION_MATRIX", "mat4", "View space to clip space transform."}, + {"PROJECTION_MATRIX", "mat4", "View space to clip space transform.", "inout"}, {"BONE_INDICES", "uvec4", "Bone indices."}, {"BONE_WEIGHTS", "vec4", "Bone weights."}, {"CUSTOM0", "vec4", "Custom value 0 (UV3/UV4)."}, {"CUSTOM1", "vec4", "Custom value 1 (UV5/UV6)."}, {"CUSTOM2", "vec4", "Custom value 2 (UV7/UV8)."}, - {"CUSTOM3", "vec4", "Custom value 3."} + {"CUSTOM3", "vec4", "Custom value 3."}, + {"Z_CLIP_SCALE", "float", "If written to, scales the vertex towards the camera to avoid clipping into things like walls. Lighting and shadows will continue to work correctly when this is written to, but screen-space effects like SSAO and SSR may break with lower scales. Try to keep this value as close to 1.0 as possible.", "out"} }; static const BuiltinList SPATIAL_FRAGMENT = { // Global + {"TIME", "float", "Global time in seconds."}, {"PI", "float", "PI constant (3.141592)."}, {"TAU", "float", "TAU constant (6.283185)."}, @@ -148,46 +154,46 @@ static const BuiltinList SPATIAL_FRAGMENT = { {"CAMERA_VISIBLE_LAYERS", "uint", "Camera cull layers."}, {"VERTEX", "vec3", "Fragment position in view space."}, - {"LIGHT_VERTEX", "vec3", "Writable VERTEX for lighting calculations (does not move pixel)."}, + {"LIGHT_VERTEX", "vec3", "Writable VERTEX for lighting calculations (does not move pixel).", "inout"}, {"VIEW_INDEX", "int", "View index."}, {"VIEW_MONO_LEFT", "int", "Constant 0."}, {"VIEW_RIGHT", "int", "Constant 1."}, {"EYE_OFFSET", "vec3", "Eye offset."}, {"SCREEN_UV", "vec2", "Screen UV coordinates."}, - {"DEPTH", "float", "Custom depth value."}, - {"NORMAL", "vec3", "Normal in view space."}, - {"TANGENT", "vec3", "Tangent in view space."}, - {"BINORMAL", "vec3", "Binormal in view space."}, - {"NORMAL_MAP", "vec3", "Normal map value."}, - {"NORMAL_MAP_DEPTH", "float", "Normal map depth (default 1.0)."}, - {"ALBEDO", "vec3", "Base color."}, - {"ALPHA", "float", "Opacity."}, - {"ALPHA_SCISSOR_THRESHOLD", "float", "Discard threshold."}, - {"ALPHA_HASH_SCALE", "float", "Alpha hash scale."}, - {"ALPHA_ANTIALIASING_EDGE", "float", "Alpha AA edge."}, - {"ALPHA_TEXTURE_COORDINATE", "vec2", "Texture coord for Alpha AA."}, - {"PREMUL_ALPHA_FACTOR", "float", "Premultiplied alpha factor."}, - {"METALLIC", "float", "Metallic (0.0 - 1.0)."}, - {"SPECULAR", "float", "Specular (0.0 - 1.0)."}, - {"ROUGHNESS", "float", "Roughness (0.0 - 1.0)."}, - {"RIM", "float", "Rim lighting."}, - {"RIM_TINT", "float", "Rim tint."}, - {"CLEARCOAT", "float", "Clearcoat intensity."}, - {"CLEARCOAT_GLOSS", "float", "Clearcoat gloss."}, - {"ANISOTROPY", "float", "Anisotropy strength."}, - {"ANISOTROPY_FLOW", "vec2", "Anisotropy direction."}, - {"SSS_STRENGTH", "float", "Subsurface scattering strength."}, - {"SSS_TRANSMITTANCE_COLOR", "vec4", "SSS Transmittance color."}, - {"SSS_TRANSMITTANCE_DEPTH", "float", "SSS Transmittance depth."}, - {"SSS_TRANSMITTANCE_BOOST", "float", "SSS Transmittance boost."}, - {"BACKLIGHT", "vec3", "Backlight color."}, - {"AO", "float", "Ambient occlusion."}, - {"AO_LIGHT_AFFECT", "float", "AO light affect."}, - {"EMISSION", "vec3", "Emission color."}, - {"FOG", "vec4", "Fog blend."}, - {"RADIANCE", "vec4", "Radiance blend."}, - {"IRRADIANCE", "vec4", "Irradiance blend."} + {"DEPTH", "float", "Custom depth value.", "out"}, + {"NORMAL", "vec3", "Normal in view space.", "inout"}, + {"TANGENT", "vec3", "Tangent in view space.", "inout"}, + {"BINORMAL", "vec3", "Binormal in view space.", "inout"}, + {"NORMAL_MAP", "vec3", "Normal map value.", "out"}, + {"NORMAL_MAP_DEPTH", "float", "Normal map depth (default 1.0).", "out"}, + {"ALBEDO", "vec3", "Base color.", "out"}, + {"ALPHA", "float", "Opacity.", "out"}, + {"ALPHA_SCISSOR_THRESHOLD", "float", "Discard threshold.", "out"}, + {"ALPHA_HASH_SCALE", "float", "Alpha hash scale.", "out"}, + {"ALPHA_ANTIALIASING_EDGE", "float", "Alpha AA edge.", "out"}, + {"ALPHA_TEXTURE_COORDINATE", "vec2", "Texture coord for Alpha AA.", "out"}, + {"PREMUL_ALPHA_FACTOR", "float", "Premultiplied alpha factor.", "out"}, + {"METALLIC", "float", "Metallic (0.0 - 1.0).", "out"}, + {"SPECULAR", "float", "Specular (0.0 - 1.0).", "out"}, + {"ROUGHNESS", "float", "Roughness (0.0 - 1.0).", "out"}, + {"RIM", "float", "Rim lighting.", "out"}, + {"RIM_TINT", "float", "Rim tint.", "out"}, + {"CLEARCOAT", "float", "Clearcoat intensity.", "out"}, + {"CLEARCOAT_GLOSS", "float", "Clearcoat gloss.", "out"}, + {"ANISOTROPY", "float", "Anisotropy strength.", "out"}, + {"ANISOTROPY_FLOW", "vec2", "Anisotropy direction.", "out"}, + {"SSS_STRENGTH", "float", "Subsurface scattering strength.", "out"}, + {"SSS_TRANSMITTANCE_COLOR", "vec4", "SSS Transmittance color.", "out"}, + {"SSS_TRANSMITTANCE_DEPTH", "float", "SSS Transmittance depth.", "out"}, + {"SSS_TRANSMITTANCE_BOOST", "float", "SSS Transmittance boost.", "out"}, + {"BACKLIGHT", "vec3", "Backlight color.", "inout"}, + {"AO", "float", "Ambient occlusion.", "out"}, + {"AO_LIGHT_AFFECT", "float", "AO light affect.", "out"}, + {"EMISSION", "vec3", "Emission color.", "out"}, + {"FOG", "vec4", "Fog blend.", "out"}, + {"RADIANCE", "vec4", "Radiance blend.", "out"}, + {"IRRADIANCE", "vec4", "Irradiance blend.", "out"} }; static const BuiltinList SPATIAL_LIGHT = { diff --git a/src/gdshader/lexer/lexer_types.h b/src/gdshader/lexer/lexer_types.h index 754a9cd..b90b490 100644 --- a/src/gdshader/lexer/lexer_types.h +++ b/src/gdshader/lexer/lexer_types.h @@ -256,7 +256,6 @@ inline std::string tokenTypeToString(TokenType t) { return "UNKNOWN_TOKEN"; } - struct Token { TokenType type; std::string value; diff --git a/src/gdshader/parser/parser.cpp b/src/gdshader/parser/parser.cpp index 6f29a8d..9f74cef 100644 --- a/src/gdshader/parser/parser.cpp +++ b/src/gdshader/parser/parser.cpp @@ -725,9 +725,8 @@ std::unique_ptr Parser::parseStatement() if (check(TokenType::TOKEN_LBRACE)) { return parseBlock(); } - - // Variable Declaration? "int x = 5;" - if (isTypeStart()) { + + if (check(TokenType::KEYWORD_CONST) || isTypeStart()) { return parseVarDecl(); } @@ -738,8 +737,12 @@ std::unique_ptr Parser::parseVarDecl() { Token start = current_token; auto node = std::make_unique(); - node->type = parseType(); + if (match(TokenType::KEYWORD_CONST)) { + node->isConst = true; + } + + node->type = parseType(); if (check(TokenType::TOKEN_IDENTIFIER)) { node->name = current_token.value; @@ -1272,7 +1275,7 @@ std::unique_ptr gdshader_lsp::Parser::parseType() // 1. Optional Precision (highp/lowp) - mostly ignored in Godot but valid syntax if (match(TokenType::KEYWORD_HIGH_PRECISION)) { node->precision = previous_token.value; - } + } // 2. Base Type Name if (isTypeStart()) { @@ -1332,6 +1335,8 @@ void gdshader_lsp::Parser::setRange(ASTNode *node, const Token &start, const Tok } bool Parser::isTypeStart() { + + SPDLOG_TRACE("Checking is type start for token {}", current_token.toString()); switch (current_token.type) { case TokenType::KEYWORD_VOID: @@ -1365,8 +1370,7 @@ bool Parser::isTypeStart() { case TokenType::TOKEN_IDENTIFIER: { - Token next = lexer.peekToken(0); // Peek next (offset 0 usually means next in queue?) - + Token next = lexer.peekToken(0); if (next.type == TokenType::TOKEN_IDENTIFIER) { return true; } diff --git a/src/gdshader/semantics/semantic_analyzer.cpp b/src/gdshader/semantics/semantic_analyzer.cpp index 1f61157..d848e9a 100644 --- a/src/gdshader/semantics/semantic_analyzer.cpp +++ b/src/gdshader/semantics/semantic_analyzer.cpp @@ -6,11 +6,11 @@ namespace gdshader_lsp { AnalysisResult SemanticAnalyzer::analyze(const ProgramNode* ast) { - symbols = SymbolTable(); // Clear + symbols = SymbolTable(); diagnostics.clear(); typeRegistry = TypeRegistry(); - currentShaderType = ShaderType::Spatial; // Reset default + currentShaderType = ShaderType::Unknown; currentProcessorFunction = ShaderStage::Global; registerGlobalFunctions(); @@ -34,15 +34,14 @@ void SemanticAnalyzer::visitShaderType(const ShaderTypeNode* node) else if (node->shaderType == "fog") currentShaderType = ShaderType::Fog; else if (node->shaderType == "spatial") currentShaderType = ShaderType::Spatial; else { - reportError(node, "Unknown shader type. Must be on of: canvas_item, particles, sky, fog, or spatial."); - currentShaderType = ShaderType::Spatial; + currentShaderType = ShaderType::Unknown; } } void SemanticAnalyzer::visitUniform(const UniformNode* node) { TypePtr type = resolveTypeFromNode(node->type.get()); - Symbol s{node->name, type, {type}, SymbolType::Uniform, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, node->hint, {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Uniform, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, node->hint, {}, {}, {}, {}, Mutability::ReadOnly}; if (!symbols.add(s)) reportError(node, "Redefinition of uniform '" + s.name + "'"); if (node->defaultValue) { @@ -123,7 +122,7 @@ void SemanticAnalyzer::visitUniform(const UniformNode* node) void SemanticAnalyzer::visitVarying(const VaryingNode* node) { TypePtr type = resolveTypeFromNode(node->type.get()); - Symbol s{node->name, type, {type}, SymbolType::Varying, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Varying, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}, {}, Mutability::ReadOnly}; if (!symbols.add(s)) reportError(node, "Redefinition of varying '" + s.name + "'"); addToken(node->nameRange.startLine, node->nameRange.startCol, @@ -132,8 +131,9 @@ void SemanticAnalyzer::visitVarying(const VaryingNode* node) void SemanticAnalyzer::visitConst(const ConstNode* node) { + SPDLOG_DEBUG("[visitConst]: Visiting a constant expression {}", node->name); TypePtr type = resolveTypeFromNode(node->type.get()); - Symbol s{node->name, type, {type}, SymbolType::Const, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}, {}, Mutability::ReadOnly}; if (!symbols.add(s)) reportError(node, "Redefinition of const '" + s.name + "'"); addToken(node->nameRange.startLine, node->nameRange.startCol, @@ -217,6 +217,9 @@ void SemanticAnalyzer::visitFunction(const FunctionNode* node) bool previousReturnFlag = currentFunctionHasReturn; TypePtr previousExpected = currentExpectedReturnType; + std::string previousFunctionName = currentFunctionName; + currentFunctionName = node->name; + if (node->name == "vertex") currentProcessorFunction = ShaderStage::Vertex; else if (node->name == "fragment") currentProcessorFunction = ShaderStage::Fragment; else if (node->name == "light") currentProcessorFunction = ShaderStage::Light; @@ -275,6 +278,8 @@ void SemanticAnalyzer::visitFunction(const FunctionNode* node) currentFunctionHasReturn = previousReturnFlag; currentExpectedReturnType = previousExpected; + currentFunctionName = previousFunctionName; + // Pop Scope int endLine = node->range.endLine; if (node->body) endLine = node->body->range.endLine; @@ -292,7 +297,6 @@ void SemanticAnalyzer::visit(const ASTNode* node) if (!node) return; if (auto p = dynamic_cast(node)) visitProgram(p); - else if (auto s = dynamic_cast(node)) visitShaderType(s); else if (auto gu = dynamic_cast(node)) addToken(gu, 3, 0); else if (auto u = dynamic_cast(node)) visitUniform(u); else if (auto v = dynamic_cast(node)) visitVarying(v); @@ -319,17 +323,19 @@ void SemanticAnalyzer::visit(const ASTNode* node) void SemanticAnalyzer::visitProgram(const ProgramNode* node) { - // First pass: Find shader_type (it affects everything else) for (const auto& child : node->nodes) { if (auto s = dynamic_cast(child.get())) { visitShaderType(s); - break; + break; } } - // Second pass: Visit everything + if (currentShaderType == ShaderType::Unknown) { + reportError(node, "Unknown shader type. Must be on of: canvas_item, particles, sky, fog, or spatial."); + return; + } + for (const auto& child : node->nodes) { - // Skip ShaderType as we already handled it if (dynamic_cast(child.get())) continue; visit(child.get()); } @@ -381,11 +387,12 @@ void gdshader_lsp::SemanticAnalyzer::visitDefine(const DefineNode *node) TypePtr type = resolveType(node->value.get()); // Register as a Const Symbol - // This allows it to be used in array sizes (float x[MAX_LIGHTS]) and math! - Symbol s{node->name, type, {type}, SymbolType::Const, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, 0, "Macro Definition", {}, {}, {}}; + // This allows it to be used in array sizes (float x[MAX_LIGHTS]) and math + Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, 0, "Macro Definition", {}, {}, {}, {}, Mutability::ReadOnly}; if (!symbols.add(s)) { // Optional: Warn about macro redefinition? + reportWarning(node, "Macro redefinition"); } } // Case 2: #define USE_FOG (Flag only) @@ -393,10 +400,9 @@ void gdshader_lsp::SemanticAnalyzer::visitDefine(const DefineNode *node) // We register flags as boolean constants so they appear in autocomplete // but typically they aren't used in expressions (unless inside #ifdef). TypePtr type = typeRegistry.getType("bool"); - Symbol s{node->name, type, {type}, SymbolType::Const, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, 0, "Preprocessor Flag", {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, 0, "Preprocessor Flag", {}, {}, {}, {}, Mutability::ReadOnly}; symbols.add(s); } - addToken(node, 3, 1); } @@ -439,9 +445,6 @@ void SemanticAnalyzer::visitBlock(const BlockNode* node) } if (sym && sym->category == SymbolType::Variable && sym->usages.empty()) { - - // We use the 'varDecl' node for the range, so the whole "int x = 0;" is highlighted. - // This allows the Code Action to delete the entire line easily. reportWarning(varDecl, "Unused variable '" + varDecl->name + "'"); } } @@ -456,20 +459,25 @@ void SemanticAnalyzer::visitVarDecl(const VariableDeclNode* node) if (type->kind == TypeKind::UNKNOWN) reportError(node, "Unknown type '" + node->type->toString() + "'"); - if (symbols.lookup(node->name)) + const Symbol* s = symbols.lookup(node->name); + if (s) { // If it exists, but add() succeeds, it means it wasn't in the *current* immediate scope // (because add() checks strict redefinition). Therefore, it must be shadowing. // We defer the warning until we know add() succeeds. - - Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}, {}, (node->isConst ? Mutability::ReadOnly : Mutability::Mutable)}; if (symbols.add(s)) { - reportWarning(node, "Variable '" + node->name + "' shadows an existing declaration."); + + const std::vector all_symbols = symbols.lookup_all(node->name); + for (const Symbol* sym : all_symbols) { + if (sym->category == SymbolType::Builtin) reportError(node, "Variable '" + node->name + "' shadows an existing builtin declaration."); + } + reportWarning(node, "Variable '" + node->name + "' shadows " + std::to_string(all_symbols.size() - 1) + " existing declaration(s)."); } else { reportError(node, "Redefinition of variable '" + node->name + "' in the same scope."); } } else { - Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}}; + Symbol s{node->name, type, {type}, SymbolType::Variable, (node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, "", {}, {}, {}, {}, (node->isConst ? Mutability::ReadOnly : Mutability::Mutable)}; symbols.add(s); } @@ -478,8 +486,7 @@ void SemanticAnalyzer::visitVarDecl(const VariableDeclNode* node) if (node->initializer) { visit(node->initializer.get()); TypePtr initType = resolveType(node->initializer.get()); - - // Smart Comparison using TypePtr + if (initType->kind != TypeKind::UNKNOWN && *initType != *type) { reportTypeMismatch(node, type->toString(), initType->toString()); } @@ -617,8 +624,8 @@ void SemanticAnalyzer::visitIdentifier(const IdentifierNode* node) else if (s->category == SymbolType::Struct) { type = 2; // Struct } - else if (s->category == SymbolType::Const) { - type = 0; + else if (s->mutability == Mutability::ReadOnly) { + type = 0; mod |= (1 << 1); // ReadOnly } else if (s->category == SymbolType::Uniform) { @@ -643,6 +650,7 @@ void SemanticAnalyzer::visitBinaryOp(const BinaryOpNode* node) if (isAssignment) { + SPDLOG_DEBUG("[BinaryOp]: Is assignment."); visitAssignment(node); return; } @@ -653,11 +661,17 @@ void SemanticAnalyzer::visitBinaryOp(const BinaryOpNode* node) TypePtr l = resolveType(node->left.get()); TypePtr r = resolveType(node->right.get()); + SPDLOG_DEBUG("[BinaryOo]: Left hand type is {}, right hand type is {}", l->toString(), r->toString()); + // If children are already "unknown", an error was likely already reported // (e.g., "Undefined identifier"). Stop here to prevent error cascading. - if (l->kind == TypeKind::UNKNOWN || r->kind == TypeKind::UNKNOWN) return; + if (l->kind == TypeKind::UNKNOWN || r->kind == TypeKind::UNKNOWN) { + SPDLOG_TRACE("Unknown type kind in binary op not reported."); + return; + } TypePtr result = getBinaryOpResultType(l, r, node->op); + SPDLOG_DEBUG("[BinaryOp]: Result type is {}", result->toString()); if (result->kind == TypeKind::UNKNOWN) { reportError(node, "Invalid binary operation '" + l->toString() + "' and '" + r->toString() + "'"); } @@ -665,44 +679,68 @@ void SemanticAnalyzer::visitBinaryOp(const BinaryOpNode* node) void SemanticAnalyzer::visitAssignment(const BinaryOpNode* node) { - if (node->right) visit(node->right.get()); + if (node->right) { + SPDLOG_DEBUG("[Assignment]: Is left side, visiting right side."); + visit(node->right.get()); + } const Symbol* s = getRootSymbol(node->left.get(), symbols); TypePtr lType = typeRegistry.getUnknownType(); + if (s) { + SPDLOG_DEBUG("[Assignment]: Root symbol is {} (builtin {}) of type {} (mutable {})", + s->name, s->category == SymbolType::Builtin, s->type->toString(), s->mutability == Mutability::Mutable); + } else { + SPDLOG_DEBUG("[Assignment]: Root symbol not found (Temporary expression or Undefined)."); + } + if (auto id = dynamic_cast(node->left.get())) { - s = symbols.lookup(id->name); + SPDLOG_DEBUG("[Assignment]: Left side node is IdentifierNode"); if (s) lType = s->type; else reportError(node, "Undefined '" + id->name + "'"); } else if (dynamic_cast(node->left.get())) { + SPDLOG_DEBUG("[Assignment]: Left side node is MemberAccessNode"); visit(node->left.get()); lType = resolveType(node->left.get()); + } else { + reportError(node->left.get(), "Expression is not assignable."); + return; } - // 3. Permission Checks (Const, Uniform, Varying) if (s) { - if (s->category == SymbolType::Const) { - reportError(node->left.get(), "Cannot assign to constant '" + s->name + "'"); - } - else if (s->category == SymbolType::Uniform) { - reportError(node->left.get(), "Cannot assign to uniform '" + s->name + "'"); + SPDLOG_DEBUG("[Assignment]: Checking symbol {} for access right", s->name); + + if (s->mutability == Mutability::ReadOnly) { + + std::string reason = "read-only variable"; + if (s->category == SymbolType::Variable && s->mutability == Mutability::ReadOnly) reason = "constant"; + else if (s->category == SymbolType::Uniform) reason = "uniform"; + else if (s->category == SymbolType::Builtin) reason = "built-in input"; + else if (s->category == SymbolType::Varying) reason = "varying"; + + reportError(node->left.get(), "Cannot assign to " + reason + " '" + s->name + "'"); } + else if (s->category == SymbolType::Varying && currentProcessorFunction != ShaderStage::Vertex) { reportError(node->left.get(), "Varyings are read-only in the fragment processor."); } } TypePtr rType = resolveType(node->right.get()); - if (lType->kind == TypeKind::UNKNOWN || rType->kind == TypeKind::UNKNOWN) return; + + SPDLOG_DEBUG("[Assignment]: Left hand type is {}, right hand type is {}", lType->toString(), rType->toString()); + + if (lType->kind == TypeKind::UNKNOWN || rType->kind == TypeKind::UNKNOWN) { + SPDLOG_TRACE("[Assignment]: Left hand or right hand type is unknown, already reported."); + return; + } // 3. Logic for Assignment vs Compound Assignment if (node->op == TokenType::TOKEN_EQUAL) { - // Simple Assignment (=) - // Strict match or safe implicit conversion (e.g. float = int) int cost = getConversionCost(rType, lType); if (cost == -1) { - reportError(node, "Type mismatch: Cannot assign '" + rType->toString() + - "' to '" + lType->toString() + "'"); + reportError(node, "Type mismatch: Cannot assign '" + rType->toString() + + "' to '" + lType->toString() + "'"); } } else { @@ -768,8 +806,6 @@ void SemanticAnalyzer::visitAssignment(const BinaryOpNode* node) } } } - - } void gdshader_lsp::SemanticAnalyzer::visitArrayAccess(const ArrayAccessNode *node) @@ -802,14 +838,17 @@ void SemanticAnalyzer::visitUnaryOp(const UnaryOpNode* node) { */ void SemanticAnalyzer::visitFunctionCall(const FunctionCallNode* node) { + std::string name = node->functionName; + if (name == currentFunctionName) { + reportError(node, "Recursion is not allowed in shaders (function '" + node->functionName + "' calls itself)."); + } + std::vector argTypes; for (const auto& arg : node->arguments) { visit(arg.get()); argTypes.push_back(resolveType(arg.get())); } - std::string name = node->functionName; - // (If the name matches a known type, it's a constructor) if (typeRegistry.getType(name)->kind != TypeKind::UNKNOWN) { validateConstructor(node, name); @@ -890,10 +929,8 @@ void SemanticAnalyzer::visitMemberAccess(const MemberAccessNode* node) TypePtr baseT = resolveType(node->base.get()); if (baseT->kind == TypeKind::UNKNOWN) return; - - // FIX: Allow .length() on arrays if (baseT->kind == TypeKind::ARRAY && node->member == "length") { - return; // Valid + return; } addToken(node, 6, 0); @@ -1065,6 +1102,32 @@ void SemanticAnalyzer::validateConstructor(const FunctionCallNode* node, const s } } + if (target->kind == TypeKind::SCALAR) { + + if (typeName == "void") { + reportError(node, "Cannot construct 'void'."); + return; + } + + bool isBasic = (typeName == "float" || typeName == "int" || typeName == "uint" || typeName == "bool"); + if (!isBasic) { + reportError(node, "Cannot construct opaque type '" + typeName + "'."); + return; + } + + if (node->arguments.size() != 1) { + reportError(node, "Scalar constructor expects exactly 1 argument."); + } + + if (!node->arguments.empty()) { + TypePtr argT = resolveType(node->arguments[0].get()); + if (argT->kind == TypeKind::UNKNOWN) { + reportError(node->arguments[0].get(), "Invalid argument."); + } + } + return; + } + reportError(node, "Cannot construct type '" + typeName + "'."); } bool gdshader_lsp::SemanticAnalyzer::isProcessorFunction(const std::string &name) @@ -1116,12 +1179,15 @@ TypePtr gdshader_lsp::SemanticAnalyzer::getBinaryOpResultType(TypePtr l, TypePtr return typeRegistry.getType("bool"); } - // --- 2. Arithmetic Exact Match (int+int, vec3+vec3, mat4*mat4) --- + if (l == typeRegistry.getType("bool") || r == typeRegistry.getType("bool")) { + SPDLOG_DEBUG("[BinaryOp]: Binary operation using bools detected."); + return typeRegistry.getUnknownType(); + } + if (*l == *r) { return l; } - // --- 3. Vector-Scalar Mixing (vec3 * 2.0) --- bool lVec = (l->kind == TypeKind::VECTOR); bool rVec = (r->kind == TypeKind::VECTOR); bool lScalar = (l->kind == TypeKind::SCALAR); @@ -1141,10 +1207,10 @@ TypePtr gdshader_lsp::SemanticAnalyzer::getBinaryOpResultType(TypePtr l, TypePtr // Matrix * Scalar (Component-wise) if ((lMat && rScalar) || (rMat && lScalar)) { - // Assuming float scalar for matrices - if ((lScalar && l->name == "float") || (rScalar && r->name == "float")) { - return lMat ? l : r; - } + // Assuming float scalar for matrices + if ((lScalar && l->name == "float") || (rScalar && r->name == "float")) { + return lMat ? l : r; + } } // Matrix * Vector (Linear Algebra: Transform) @@ -1234,6 +1300,15 @@ const Symbol *gdshader_lsp::SemanticAnalyzer::getRootSymbol(const ExpressionNode if (auto id = dynamic_cast(node)) { return symbols.lookup(id->name); } + + if (auto mem = dynamic_cast(node)) { + return getRootSymbol(mem->base.get(), symbols); + } + + if (auto arr = dynamic_cast(node)) { + return getRootSymbol(arr->base.get(), symbols); + } + return nullptr; } @@ -1291,7 +1366,12 @@ void SemanticAnalyzer::loadBuiltinsForFunction(const std::string& funcName) const auto& builtins = get_builtins(currentShaderType, scope); for (const auto& b : builtins) { TypePtr t = typeRegistry.getType(b.type); - symbols.add({b.name, t, {t}, SymbolType::Builtin, -1, 0, b.doc, {}, {}, {}}); + + Mutability m = Mutability::Mutable; + if (b.qualifier == "in" || b.qualifier == "const") { + m = Mutability::ReadOnly; + } + symbols.add({b.name, t, {t}, SymbolType::Builtin, -1, 0, b.doc, {}, {}, {}, {}, m}); } } } @@ -1409,6 +1489,7 @@ void SemanticAnalyzer::reportError(const ASTNode* node, const std::string& msg) if (node) { int len = getNodeLength(node); diagnostics.push_back({(node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, msg, DiagnosticLevel::Error, len}); + SPDLOG_DEBUG("Reporting error: {}", msg); } } @@ -1416,6 +1497,7 @@ void SemanticAnalyzer::reportWarning(const ASTNode* node, const std::string& msg if (node) { int len = getNodeLength(node); diagnostics.push_back({(node->range.startLine > 0) ? node->range.startLine - 1 : 0, node->range.startCol, msg, DiagnosticLevel::Warning, len}); + SPDLOG_DEBUG("Reporting warning: {}", msg); } } diff --git a/src/gdshader/semantics/semantic_analyzer.hpp b/src/gdshader/semantics/semantic_analyzer.hpp index 5948ddf..6a215f9 100644 --- a/src/gdshader/semantics/semantic_analyzer.hpp +++ b/src/gdshader/semantics/semantic_analyzer.hpp @@ -31,10 +31,11 @@ class SemanticAnalyzer { std::unordered_set processedFiles; std::string currentFilePath; - ShaderType currentShaderType = ShaderType::Spatial; // Default + ShaderType currentShaderType = ShaderType::Unknown; // Default ShaderStage currentProcessorFunction = ShaderStage::Global; TypePtr currentExpectedReturnType = std::shared_ptr(); + std::string currentFunctionName = ""; bool currentFunctionHasReturn = false; void visit(const ASTNode* node); diff --git a/src/gdshader/semantics/symbol_table.cpp b/src/gdshader/semantics/symbol_table.cpp index d5a1da6..ebe0f0d 100644 --- a/src/gdshader/semantics/symbol_table.cpp +++ b/src/gdshader/semantics/symbol_table.cpp @@ -123,25 +123,39 @@ void gdshader_lsp::SymbolTable::addReference(const Symbol* sym, int line, int co } } -const Symbol* SymbolTable::lookup(const std::string& name) const +const Symbol* SymbolTable::lookup(const std::string& name, const int depth) const { Scope* walker = current; - int depth = 0; + int rdepth = 0; while (walker) { auto it = walker->symbols.find(name); if (it != walker->symbols.end() && !it->second.empty()) { - // Return the first one found. - // For variables, this is the only one. - // For functions, this returns one of the overloads (usually the first defined). return &it->second[0]; } + + rdepth++; + if (depth > 0 && rdepth >= depth) break; + walker = walker->parent; - depth++; } - SPDLOG_TRACE("[SymTable] Lookup Failed: '{}' (checked {} scopes)", name, depth); + SPDLOG_DEBUG("Lookup Failed: '{}' with depth {} (checked {} scopes)", name, depth, rdepth); return nullptr; } +const std::vector gdshader_lsp::SymbolTable::lookup_all(const std::string &name) const +{ + std::vector symbols; + Scope* walker = current; + while (walker) { + auto it = walker->symbols.find(name); + if (it != walker->symbols.end() && !it->second.empty()) { + symbols.push_back(&it->second[0]); + } + walker = walker->parent; + } + return symbols; +} + std::vector SymbolTable::lookupFunctions(const std::string& name) const { Scope* walker = current; diff --git a/src/gdshader/semantics/symbol_table.hpp b/src/gdshader/semantics/symbol_table.hpp index c90ecf7..7f73d8e 100644 --- a/src/gdshader/semantics/symbol_table.hpp +++ b/src/gdshader/semantics/symbol_table.hpp @@ -11,16 +11,22 @@ namespace gdshader_lsp { -enum class SymbolType { +enum class SymbolType +{ Variable, Uniform, Varying, - Const, Function, Struct, Builtin }; +enum class Mutability { + Mutable, + ReadOnly, + WriteOnly +}; + struct Symbol { std::string name; @@ -43,8 +49,8 @@ struct Symbol }; mutable std::vector usages; - bool is_definition = false; + Mutability mutability = Mutability::Mutable; }; struct Scope @@ -88,11 +94,18 @@ class SymbolTable { /** * @brief With overloading, this is mainly used for local variable lookup, where we retrieve the first symbol matched. - * * @param name * @return const Symbol* */ - const Symbol* lookup(const std::string& name) const; + const Symbol* lookup(const std::string& name, const int depth = 0) const; + + /** + * @brief Returns all symbols that match the symbol name across ALL scopes. + * @param name + * @return const std::vector + */ + const std::vector lookup_all(const std::string& name) const; + std::vector lookupFunctions(const std::string& name) const; const Scope* findScopeAt(int line) const; diff --git a/tests/test_diagnostics_batch.py b/tests/test_diagnostics_batch.py new file mode 100644 index 0000000..7d9496b --- /dev/null +++ b/tests/test_diagnostics_batch.py @@ -0,0 +1,268 @@ +import pytest # type: ignore + +# List of test cases: (Name, Shader Code, Expected Error Substring) +ERROR_CASES = [ + + # Basic syntax + ( + "missing_semicolon", + """ + shader_type spatial; + void fragment() { + float x = 1.0 + } + """, + "expected ';'" + ), + ( + "unbalanced_braces", + """ + shader_type spatial; + void fragment() { + if (true) { + float x = 1.0; + // Missing closing brace + } + """, + "}" + ), + + # Type safety + + ( + "assign_int_to_vec", + """ + shader_type spatial; + void fragment() { + vec3 v = 1; + } + """, + "Type mismatch" + ), + ( + "too_few_args", + """ + shader_type spatial; + void fragment() { + vec3 v = vec3(1.0, 2.0); + } + """, + "components" + ), + ( + "assign_int_to_float", + """ + shader_type spatial; + void fragment() { + float f = 1; + } + """, + "Type mismatch" + ), + ( + "assign_vec3_to_float", + """ + shader_type spatial; + void fragment() { + float f = vec3(1.0); + } + """, + "Type mismatch" + ), + ( + "bool_arithmetic", + """ + shader_type spatial; + void fragment() { + bool a = true; + bool b = false; + int c = a + b; + } + """, + "Invalid binary operation" + ), + + # Vector and swizzle logic + + ( + "swizzle_invalid_component", + """ + shader_type spatial; + void fragment() { + vec3 v = vec3(1.0); + float f = v.q; + } + """, + "swizzle" # or "member" + ), + ( + "swizzle_out_of_bounds", + """ + shader_type spatial; + void fragment() { + vec2 v = vec2(1.0); + float f = v.z; + } + """, + "swizzle" + ), + ( + "swizzle_assignment_mismatch", + """ + shader_type spatial; + void fragment() { + vec3 v = vec3(1.0); + v.xy = vec3(1.0); + } + """, + "assign" # "Cannot assign 'vec3' to 'vec2'" + ), + + # Shader stage validation + + ( + "discard_in_vertex", + """ + shader_type spatial; + void vertex() { + discard; + } + """, + "discard" # "discard only allowed in fragment shader" + ), + ( + "writing_albedo_in_vertex", + """ + shader_type spatial; + void vertex() { + ALBEDO = vec3(1.0); + } + """, + "Undefined" # or "ALBEDO" + ), + + # Qualifiers and constants + + ( + "write_to_uniform", + """ + shader_type spatial; + uniform float u_time; + void fragment() { + u_time = 10.0; + } + """, + "Cannot assign" # or "assign" + ), + ( + "write_to_const", + """ + shader_type spatial; + void fragment() { + const float PI = 3.14; + PI = 3.14159; + } + """, + "constant" # or "read-only" + ), + ( + "opaque_type_construction", + """ + shader_type spatial; + void fragment() { + sampler2D s = sampler2D(1.0); + } + """, + "construct" + ), + + # Functions and control flow + + ( + "void_function_return_value", + """ + shader_type spatial; + void do_nothing() { + } + void fragment() { + float x = do_nothing(); + } + """, + "void" + ), + ( + "missing_return", + """ + shader_type spatial; + float get_val() { + + } + void fragment() { + } + """, + "return" + ), + ( + "recursion_check", + """ + shader_type spatial; + void recursive() { + recursive(); + } + void fragment() { recursive(); } + """, + "recursion" + ), + ( + "missing_shader_type", + """ + # Missing shader_type at the top! + void fragment() { + ALBEDO = vec3(1.0); + } + """, + "Unknown shader type" + ) + +] + +@pytest.mark.parametrize("name, code, expected_error", ERROR_CASES) +def test_error_messages_specifically(lsp, name, code, expected_error): + """ + Data-driven test to verify specific error messages. + Useful for refining error text clarity. + """ + + # Create unique URI for each case to avoid caching issues + uri = f"file:///error_{name}.gdshader" + + lsp.send_message("textDocument/didOpen", { + "textDocument": { + "uri": uri, + "languageId": "gdshader", + "version": 1, + "text": code + } + }, is_notification=True) + + res = lsp.read_message() + + # Assert we got diagnostics + assert res["method"] == "textDocument/publishDiagnostics" + diags = res["params"]["diagnostics"] + + if len(diags) == 0: + pytest.fail(f"Expected error containing '{expected_error}', but got valid compilation.") + + # Check if ANY of the diagnostics contain our expected string + found = False + messages = [] + for d in diags: + msg = d["message"] + messages.append(msg) + if expected_error.lower() in msg.lower(): + found = True + break + + if not found: + pytest.fail(f"Expected error to contain '{expected_error}'. Got: {messages}") \ No newline at end of file diff --git a/tests/test_exstended.py b/tests/test_exstended.py new file mode 100644 index 0000000..d8968a2 --- /dev/null +++ b/tests/test_exstended.py @@ -0,0 +1,108 @@ +import pytest # type: ignore + +# ------------------------------------------------------------------------- +# TEST 7: HOVER DOCUMENTATION +# ------------------------------------------------------------------------- +def test_hover_builtin(lsp): + # Test that hovering over a built-in function returns documentation + shader_code = """ + shader_type spatial; + void fragment() { + float x = sin(1.0); + } + """ + + lsp.send_message("textDocument/didOpen", { + "textDocument": { + "uri": "file:///hover.gdshader", + "languageId": "gdshader", + "version": 1, + "text": shader_code + } + }, is_notification=True) + lsp.read_message() # Consume diagnostics + + # Hover over 'sin' (Line 3, Char 19 approx) + msg_id = lsp.send_message("textDocument/hover", { + "textDocument": {"uri": "file:///hover.gdshader"}, + "position": {"line": 3, "character": 19} + }) + + res = lsp.read_message() + assert res["id"] == msg_id + + # We expect a result containing "sin" or description + # Result format: { contents: "markdown string" } or { contents: { kind: "markdown", value: "..." } } + result = res["result"] + assert result is not None + + contents = result["contents"] + value = contents["value"] if isinstance(contents, dict) else contents + + assert "sin" in value or "sine" in value.lower() + +# ------------------------------------------------------------------------- +# TEST 8: DOCUMENT SYMBOLS (OUTLINE) +# ------------------------------------------------------------------------- +def test_document_symbols(lsp): + # Does the LSP provide a list of functions/structs in the file? + shader_code = """ + shader_type spatial; + uniform float speed; + void vertex() {} + void fragment() {} + """ + + lsp.send_message("textDocument/didOpen", { + "textDocument": { + "uri": "file:///outline.gdshader", + "languageId": "gdshader", + "version": 1, + "text": shader_code + } + }, is_notification=True) + lsp.read_message() + + msg_id = lsp.send_message("textDocument/documentSymbol", { + "textDocument": {"uri": "file:///outline.gdshader"} + }) + + res = lsp.read_message() + symbols = res["result"] + + names = [s["name"] for s in symbols] + assert "vertex" in names + assert "fragment" in names + # Optional: Check if 'speed' uniform is listed + # assert "speed" in names + +# ------------------------------------------------------------------------- +# TEST 9: SCOPE VALIDATION +# ------------------------------------------------------------------------- +def test_variable_scope_rules(lsp): + # Variables defined inside a block {} should not be visible outside + shader_code = """ + shader_type spatial; + void fragment() { + if (true) { + float hidden = 1.0; + } + ALBEDO = vec3(hidden); // Error expected here + } + """ + + lsp.send_message("textDocument/didOpen", { + "textDocument": { + "uri": "file:///scope.gdshader", + "languageId": "gdshader", + "version": 1, + "text": shader_code + } + }, is_notification=True) + + res = lsp.read_message() + diags = res["params"]["diagnostics"] + + assert len(diags) > 0 + # Error should be about undefined identifier 'hidden' + assert "hidden" in diags[0]["message"] or "undeclared" in diags[0]["message"] \ No newline at end of file