Merge pull request #983 from BradLewis/feat/improve-semantic-types

Feat/improve semantic types

7 files changed, 126 insertions, 91 deletions

diff --git a/src/server/analysis.odin b/src/server/analysis.odin
index 5985fea..1b50468 100644
--- a/src/server/analysis.odin
+++ b/src/server/analysis.odin
@@ -1387,7 +1387,8 @@ resolve_selector_expression :: proc(ast_context: ^AstContext, node: ^ast.Selecto
 					set_ast_package_from_node_scoped(ast_context, s.types[i])
 					ast_context.field_name = node.field^
 					ok := internal_resolve_type_expression(ast_context, s.types[i], &symbol)
-					symbol.type = .Variable
+					symbol.type = .Field
+					symbol.flags |= {.Mutable}
 					return symbol, ok
 				}
 			}
@@ -1396,7 +1397,8 @@ resolve_selector_expression :: proc(ast_context: ^AstContext, node: ^ast.Selecto
 				if node.field != nil && name == node.field.name {
 					ast_context.field_name = node.field^
 					ok := internal_resolve_type_expression(ast_context, s.types[i], &symbol)
-					symbol.type = .Variable
+					symbol.type = .Field
+					symbol.flags |= {.Mutable}
 					return symbol, ok
 				}
 			}
@@ -1682,7 +1684,7 @@ resolve_local_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, loca
 	case ^ast.Basic_Lit:
 		return_symbol, ok = resolve_basic_lit(ast_context, v^)
 		return_symbol.name = node.name
-		return_symbol.type = local.variable ? .Variable : .Constant
+		return_symbol.type = .Mutable in local.flags ? .Variable : .Constant
 	case ^ast.Binary_Expr:
 		return_symbol, ok = resolve_binary_expression(ast_context, v)
 	case:
@@ -1698,8 +1700,12 @@ resolve_local_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, loca
 		return_symbol.flags |= {.Parameter}
 	}
 
-	if local.variable {
+	if .Mutable in local.flags {
 		return_symbol.type = .Variable
+		return_symbol.flags |= {.Mutable}
+	}
+	if .Variable in local.flags {
+		return_symbol.flags |= {.Variable}
 	}
 
 	return_symbol.flags |= {.Local}
@@ -1733,7 +1739,7 @@ resolve_global_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, glo
 		}
 
 		if ok = internal_resolve_type_expression(ast_context, v.expr, &return_symbol); ok {
-			return_types := get_proc_return_types(ast_context, return_symbol, v, global.mutable)
+			return_types := get_proc_return_types(ast_context, return_symbol, v, .Mutable in global.flags)
 			if len(return_types) > 0 {
 				ok = internal_resolve_type_expression(ast_context, return_types[0], &return_symbol)
 			}
@@ -1799,7 +1805,7 @@ resolve_global_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, glo
 	case ^ast.Basic_Lit:
 		return_symbol, ok = resolve_basic_lit(ast_context, v^)
 		return_symbol.name = node.name
-		return_symbol.type = global.mutable ? .Variable : .Constant
+		return_symbol.type = .Mutable in global.flags ? .Variable : .Constant
 	case:
 		ok = internal_resolve_type_expression(ast_context, global.expr, &return_symbol)
 	}
@@ -1809,8 +1815,13 @@ resolve_global_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, glo
 		return_symbol.flags |= {.Distinct}
 	}
 
-	if global.mutable {
+	if .Mutable in global.flags {
 		return_symbol.type = .Variable
+		return_symbol.flags |= {.Mutable}
+	}
+
+	if .Variable in global.flags {
+		return_symbol.flags |= {.Variable}
 	}
 
 	if global.docs != nil {
@@ -2637,6 +2648,7 @@ resolve_container_allocator_location :: proc(ast_context: ^AstContext, container
 				for name, i in v.names {
 					if name == "allocator" {
 						symbol.range = v.ranges[i]
+						symbol.type = .Field
 						return symbol, true
 					}
 				}
@@ -2683,18 +2695,21 @@ resolve_symbol_selector :: proc(
 		for name, i in v.names {
 			if strings.compare(name, field) == 0 {
 				symbol.range = v.ranges[i]
+				symbol.type = .EnumMember
 			}
 		}
 	case SymbolStructValue:
 		for name, i in v.names {
 			if strings.compare(name, field) == 0 {
 				symbol.range = v.ranges[i]
+				symbol.type = .Field
 			}
 		}
 	case SymbolBitFieldValue:
 		for name, i in v.names {
 			if strings.compare(name, field) == 0 {
 				symbol.range = v.ranges[i]
+				symbol.type = .Field
 			}
 		}
 	case SymbolPackageValue:
diff --git a/src/server/ast.odin b/src/server/ast.odin
index e658123..2580338 100644
--- a/src/server/ast.odin
+++ b/src/server/ast.odin
@@ -82,11 +82,16 @@ are_keyword_aliases :: proc(a, b: string) -> bool {
 	return false
 }
 
+GlobalFlags :: enum {
+	Mutable, // or constant
+	Variable, // or type
+}
+
 GlobalExpr :: struct {
 	name:       string,
 	name_expr:  ^ast.Expr,
 	expr:       ^ast.Expr,
-	mutable:    bool,
+	flags:      bit_set[GlobalFlags],
 	docs:       ^ast.Comment_Group,
 	comment:    ^ast.Comment_Group,
 	attributes: []^ast.Attribute,
@@ -353,6 +358,17 @@ merge_attributes :: proc(attrs: []^ast.Attribute, foreign_attrs: []^ast.Attribut
 	return new_attrs[:]
 }
 
+// TODO: it seems the global symbols don't distinguish between a type decl and
+// a const variable declaration, so we do a quick check here to distinguish the cases.
+is_variable_declaration :: proc(expr: ^ast.Expr) -> bool {
+	#partial switch v in expr.derived {
+	case ^ast.Comp_Lit, ^ast.Basic_Lit, ^ast.Type_Cast, ^ast.Call_Expr:
+		return true
+	case:
+		return false
+	}
+}
+
 collect_value_decl :: proc(
 	exprs: ^[dynamic]GlobalExpr,
 	file: ast.File,
@@ -370,13 +386,16 @@ collect_value_decl :: proc(
 	attributes := merge_attributes(value_decl.attributes[:], foreign_attrs)
 
 	global_expr := GlobalExpr {
-		mutable    = value_decl.is_mutable,
 		docs       = value_decl.docs,
 		comment    = comment,
 		attributes = attributes,
 		private    = file_tags.private,
 	}
 
+	if value_decl.is_mutable {
+		global_expr.flags += {.Mutable}
+	}
+
 	for attribute in attributes {
 		for elem in attribute.elems {
 			ident, value, ok := unwrap_attr_elem(elem)
@@ -414,6 +433,11 @@ collect_value_decl :: proc(
 		global_expr.name = get_ast_node_string(name, file.src)
 		global_expr.name_expr = name
 
+		if len(value_decl.values) > i {
+			if is_variable_declaration(value_decl.values[i]) {
+				global_expr.flags += {.Variable}
+			}
+		}
 		if value_decl.type != nil {
 			global_expr.expr = value_decl.type
 			append(exprs, global_expr)
@@ -477,13 +501,7 @@ collect_when_body :: proc(
 			if foreign_decl.body != nil {
 				if foreign_block, ok := foreign_decl.body.derived.(^ast.Block_Stmt); ok {
 					for foreign_stmt in foreign_block.stmts {
-						collect_value_decl(
-							exprs,
-							file,
-							file_tags,
-							foreign_stmt,
-							foreign_decl.attributes[:],
-						)
+						collect_value_decl(exprs, file, file_tags, foreign_stmt, foreign_decl.attributes[:])
 					}
 				}
 			}
@@ -732,8 +750,8 @@ free_ast_node :: proc(node: ^ast.Node, allocator: mem.Allocator) {
 		free_ast(n.names, allocator)
 		free_ast(n.type, allocator)
 		free_ast(n.default_value, allocator)
-		free_ast_comment(n.docs, allocator);
-		free_ast_comment(n.comment, allocator);
+		free_ast_comment(n.docs, allocator)
+		free_ast_comment(n.comment, allocator)
 	case ^Field_List:
 		free_ast(n.list, allocator)
 	case ^Typeid_Type:
@@ -1306,7 +1324,12 @@ construct_struct_field_docs :: proc(file: ast.File, v: ^ast.Struct_Type, allocat
 						field.comment.list = list[:1]
 					}
 					if len(list) > 1 {
-						next_field.docs = new_type(ast.Comment_Group, list[1].pos, parser.end_pos(list[len(list) - 2]), allocator)
+						next_field.docs = new_type(
+							ast.Comment_Group,
+							list[1].pos,
+							parser.end_pos(list[len(list) - 2]),
+							allocator,
+						)
 						next_field.docs.list = list[1:]
 					} else {
 						next_field.docs = nil
diff --git a/src/server/collector.odin b/src/server/collector.odin
index 4588a39..a702546 100644
--- a/src/server/collector.odin
+++ b/src/server/collector.odin
@@ -663,7 +663,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 			token = v^
 			symbol.value = collect_generic(collection, col_expr, package_map, uri)
 
-			if expr.mutable {
+			if .Mutable in expr.flags {
 				token_type = .Variable
 			} else {
 				token_type = .Unresolved
@@ -671,7 +671,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 		case ^ast.Comp_Lit:
 			generic := collect_generic(collection, col_expr, package_map, uri)
 
-			if expr.mutable {
+			if .Mutable in expr.flags {
 				token_type = .Variable
 			} else {
 				token_type = .Unresolved
@@ -685,7 +685,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 			// default
 			symbol.value = collect_generic(collection, col_expr, package_map, uri)
 
-			if expr.mutable {
+			if .Mutable in expr.flags {
 				token_type = .Variable
 			} else {
 				token_type = .Unresolved
@@ -734,6 +734,10 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 			symbol.flags |= {.PrivatePackage}
 		}
 
+		if .Variable in expr.flags {
+			symbol.flags |= {.Variable}
+		}
+
 
 		pkg: ^SymbolPackage
 		ok: bool
diff --git a/src/server/completion.odin b/src/server/completion.odin
index e9b4de3..88bdfdd 100644
--- a/src/server/completion.odin
+++ b/src/server/completion.odin
@@ -747,6 +747,7 @@ get_selector_completion :: proc(
 	}
 
 	if selector.type != .Variable &&
+	   selector.type != .Field &&
 	   selector.type != .Package &&
 	   selector.type != .Enum &&
 	   selector.type != .Function {
diff --git a/src/server/locals.odin b/src/server/locals.odin
index 267a2a2..63a1452 100644
--- a/src/server/locals.odin
+++ b/src/server/locals.odin
@@ -4,6 +4,11 @@ import "core:fmt"
 import "core:log"
 import "core:odin/ast"
 
+LocalFlag :: enum {
+	Mutable, // or constant
+	Variable, // or type
+}
+
 DocumentLocal :: struct {
 	lhs:             ^ast.Expr,
 	rhs:             ^ast.Expr,
@@ -11,7 +16,7 @@ DocumentLocal :: struct {
 	resolved_global: bool, //Some locals have already been resolved and are now in global space
 	local_global:    bool, //Some locals act like globals, i.e. functions defined inside functions.
 	pkg:             string,
-	variable:        bool,
+	flags:           bit_set[LocalFlag],
 	parameter:       bool,
 }
 
@@ -25,7 +30,7 @@ store_local :: proc(
 	name: string,
 	local_global: bool,
 	resolved_global: bool,
-	variable: bool,
+	flags: bit_set[LocalFlag],
 	pkg: string,
 	parameter: bool,
 ) {
@@ -46,7 +51,7 @@ store_local :: proc(
 			resolved_global = resolved_global,
 			local_global = local_global,
 			pkg = pkg,
-			variable = variable,
+			flags = flags,
 			parameter = parameter,
 		},
 	)
@@ -297,6 +302,11 @@ get_locals_value_decl :: proc(file: ast.File, value_decl: ast.Value_Decl, ast_co
 	if value_decl.type != nil {
 		for name, i in value_decl.names {
 			str := get_ast_node_string(value_decl.names[i], file.src)
+			flags: bit_set[LocalFlag]
+			if value_decl.is_mutable {
+				flags |= {.Mutable}
+
+			}
 			store_local(
 				ast_context,
 				name,
@@ -305,7 +315,7 @@ get_locals_value_decl :: proc(file: ast.File, value_decl: ast.Value_Decl, ast_co
 				str,
 				ast_context.non_mutable_only,
 				false,
-				value_decl.is_mutable,
+				flags,
 				"",
 				false,
 			)
@@ -333,16 +343,25 @@ get_locals_value_decl :: proc(file: ast.File, value_decl: ast.Value_Decl, ast_co
 	for name, i in value_decl.names {
 		result_i := min(len(results) - 1, i)
 		str := get_ast_node_string(name, file.src)
+		flags: bit_set[LocalFlag]
+
+		expr := results[result_i]
+		if is_variable_declaration(expr) {
+			flags |= {.Variable}
+		}
+		if value_decl.is_mutable {
+			flags |= {.Mutable}
+		}
 
 		store_local(
 			ast_context,
 			name,
-			results[result_i],
+			expr,
 			value_decl.end.offset,
 			str,
 			ast_context.non_mutable_only,
 			false, // calls[result_i] or_else false, // TODO: find a good way to handle this
-			value_decl.is_mutable,
+			flags,
 			get_package_from_node(results[result_i]^),
 			false,
 		)
@@ -479,7 +498,7 @@ get_locals_using :: proc(expr: ^ast.Expr, ast_context: ^AstContext) {
 				selector.expr = expr
 				selector.field = new_type(ast.Ident, v.types[i].pos, v.types[i].end, ast_context.allocator)
 				selector.field.name = name
-				store_local(ast_context, expr, selector, 0, name, false, ast_context.non_mutable_only, true, "", false)
+				store_local(ast_context, expr, selector, 0, name, false, ast_context.non_mutable_only, {.Mutable}, "", false)
 			}
 		case SymbolBitFieldValue:
 			for name, i in v.names {
@@ -487,7 +506,7 @@ get_locals_using :: proc(expr: ^ast.Expr, ast_context: ^AstContext) {
 				selector.expr = expr
 				selector.field = new_type(ast.Ident, v.types[i].pos, v.types[i].end, ast_context.allocator)
 				selector.field.name = name
-				store_local(ast_context, expr, selector, 0, name, false, ast_context.non_mutable_only, true, "", false)
+				store_local(ast_context, expr, selector, 0, name, false, ast_context.non_mutable_only, {.Mutable}, "", false)
 			}
 		}
 	}
@@ -527,7 +546,7 @@ get_locals_assign_stmt :: proc(file: ast.File, stmt: ast.Assign_Stmt, ast_contex
 				ident.name,
 				ast_context.non_mutable_only,
 				false,
-				true,
+				{.Mutable},
 				"",
 				false,
 			)
@@ -580,7 +599,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
 						false,
 					)
@@ -618,7 +637,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -636,7 +655,7 @@ get_locals_for_range_stmt :: proc(
 							ident.name,
 							ast_context.non_mutable_only,
 							false,
-							true,
+							{.Mutable},
 							symbol.pkg,
 							false,
 						)
@@ -655,7 +674,7 @@ get_locals_for_range_stmt :: proc(
 							ident.name,
 							ast_context.non_mutable_only,
 							false,
-							true,
+							{.Mutable},
 							symbol.pkg,
 							false,
 						)
@@ -673,7 +692,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -689,7 +708,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -706,7 +725,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -722,7 +741,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -739,7 +758,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -759,7 +778,7 @@ get_locals_for_range_stmt :: proc(
 								ident.name,
 								ast_context.non_mutable_only,
 								false,
-								true,
+								{.Mutable},
 								len_symbol.pkg,
 								false,
 							)
@@ -773,7 +792,7 @@ get_locals_for_range_stmt :: proc(
 							ident.name,
 							ast_context.non_mutable_only,
 							false,
-							true,
+							{.Mutable},
 							symbol.pkg,
 							false,
 						)
@@ -791,7 +810,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -807,7 +826,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -824,7 +843,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -840,7 +859,7 @@ get_locals_for_range_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						symbol.pkg,
 						false,
 					)
@@ -914,7 +933,7 @@ get_locals_type_switch_stmt :: proc(
 						ident.name,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
 						false,
 					)
@@ -953,7 +972,7 @@ get_locals_proc_param_and_results :: proc(
 						str,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
 						true,
 					)
@@ -974,7 +993,7 @@ get_locals_proc_param_and_results :: proc(
 						str,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
 						true,
 					)
@@ -996,9 +1015,9 @@ get_locals_proc_param_and_results :: proc(
 						str,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
-						true,
+						false,
 					)
 				} else {
 					str := get_ast_node_string(name, file.src)
@@ -1010,9 +1029,9 @@ get_locals_proc_param_and_results :: proc(
 						str,
 						ast_context.non_mutable_only,
 						false,
-						true,
+						{.Mutable},
 						"",
-						true,
+						false,
 					)
 				}
 			}
diff --git a/src/server/semantic_tokens.odin b/src/server/semantic_tokens.odin
index 7210ccf..7d6b653 100644
--- a/src/server/semantic_tokens.odin
+++ b/src/server/semantic_tokens.odin
@@ -390,44 +390,14 @@ visit_node :: proc(node: ^ast.Node, builder: ^SemanticTokenBuilder) {
 	}
 }
 
-// TODO: it seems the global symbols don't distinguish between a type decl and
-// a const variable declaration, so we do a quick check here to distinguish the cases.
-is_variable_declaration :: proc(expr: ^ast.Expr) -> bool {
-	#partial switch v in expr.derived {
-	case ^ast.Comp_Lit, ^ast.Basic_Lit, ^ast.Type_Cast, ^ast.Call_Expr:
-		return true
-	case:
-		return false
-	}
-}
-
 visit_value_decl :: proc(value_decl: ast.Value_Decl, builder: ^SemanticTokenBuilder) {
 	using ast
 
 	modifiers: SemanticTokenModifiers = value_decl.is_mutable ? {} : {.ReadOnly}
 
-	// Check if we are a comp lit or a type declaration
-
-	// a, b, c :: 1, 2, 3
-	// a := 1
-	if len(value_decl.values) == len(value_decl.names) {
-		for name, i in value_decl.names {
-			ident := name.derived.(^Ident) or_continue
-			is_var_decl := is_variable_declaration(value_decl.values[i])
-			visit_ident(ident, ident, modifiers, builder, is_var_decl)
-		}
-	} else if len(value_decl.values) > 0 {
-		// a, b: int
-		is_var_decl := is_variable_declaration(value_decl.values[0])
-		for name in value_decl.names {
-			ident := name.derived.(^Ident) or_continue
-			visit_ident(ident, ident, modifiers, builder, is_var_decl)
-		}
-	} else {
-		for name in value_decl.names {
-			ident := name.derived.(^Ident) or_continue
-			visit_ident(ident, ident, modifiers, builder)
-		}
+	for name in value_decl.names {
+		ident := name.derived.(^Ident) or_continue
+		visit_ident(ident, ident, modifiers, builder)
 	}
 
 	visit_node(value_decl.type, builder)
@@ -559,7 +529,6 @@ visit_ident :: proc(
 	symbol_ptr: rawptr,
 	modifiers: SemanticTokenModifiers,
 	builder: ^SemanticTokenBuilder,
-	is_variable_decl := false,
 ) {
 	using ast
 
@@ -571,14 +540,15 @@ visit_ident :: proc(
 
 	modifiers := modifiers
 
-	if symbol.type != .Variable {
+	if .Mutable not_in symbol.flags {
 		modifiers += {.ReadOnly}
 	}
 
-	if is_variable_decl {
+	if .Variable in symbol.flags {
 		write_semantic_node(builder, ident, .Variable, modifiers)
 		return
 	}
+
 	/* variable idents */
 	#partial switch symbol.type {
 	case .Variable, .Constant, .Function:
@@ -594,6 +564,8 @@ visit_ident :: proc(
 		}
 	case .EnumMember:
 		write_semantic_node(builder, ident, .EnumMember, modifiers)
+	case .Field:
+		write_semantic_node(builder, ident, .Property, modifiers)
 	case:
 		/* type idents */
 		switch v in symbol.value {
@@ -612,7 +584,7 @@ visit_ident :: proc(
 		     SymbolMapValue,
 		     SymbolMultiPointerValue,
 		     SymbolBasicValue,
-			 SymbolPolyTypeValue:
+		     SymbolPolyTypeValue:
 			write_semantic_node(builder, ident, .Type, modifiers)
 		case SymbolUntypedValue:
 		// handled by static syntax highlighting
diff --git a/src/server/symbol.odin b/src/server/symbol.odin
index 2090f3b..9e220c5 100644
--- a/src/server/symbol.odin
+++ b/src/server/symbol.odin
@@ -194,7 +194,8 @@ SymbolFlag :: enum {
 	PrivateFile,
 	PrivatePackage,
 	Anonymous, //Usually applied to structs that are defined inline inside another struct
-	Variable, //Symbols that are variable, this means their value decl was mutable
+	Variable, // or type
+	Mutable, // or constant
 	Local,
 	ObjC,
 	ObjCIsClassMethod, // should be set true only when ObjC is enabled