Merge pull request #1292 from BradLewis/feat/add-docs-to-package-hover

Add documentation to package hover info

8 files changed, 193 insertions, 54 deletions

diff --git a/src/server/analysis.odin b/src/server/analysis.odin
index 6d0cf09..67d7ef3 100644
--- a/src/server/analysis.odin
+++ b/src/server/analysis.odin
@@ -1812,7 +1812,7 @@ internal_resolve_type_identifier :: proc(ast_context: ^AstContext, node: ast.Ide
 
 			try_build_package(symbol.pkg)
 
-			return symbol, true
+			return resolve_symbol_return(ast_context, symbol)
 		}
 	}
 
@@ -1823,8 +1823,9 @@ internal_resolve_type_identifier :: proc(ast_context: ^AstContext, node: ast.Ide
 			pkg   = indexer.runtime_package,
 			value = SymbolPackageValue{},
 		}
+		try_build_package(symbol.pkg)
 
-		return symbol, true
+		return resolve_symbol_return(ast_context, symbol)
 	}
 
 	if global, ok := ast_context.globals[node.name];
@@ -1853,7 +1854,7 @@ internal_resolve_type_identifier :: proc(ast_context: ^AstContext, node: ast.Ide
 
 		try_build_package(symbol.pkg)
 
-		return symbol, true
+		return resolve_symbol_return(ast_context, symbol)
 	}
 
 	is_runtime := strings.contains(ast_context.current_package, "base/runtime")
@@ -1896,7 +1897,7 @@ resolve_local_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, loca
 					value = SymbolPackageValue{},
 				}
 
-				return symbol, true
+				return resolve_symbol_return(ast_context, symbol)
 			}
 		}
 	}
@@ -2639,6 +2640,16 @@ resolve_symbol_return :: proc(ast_context: ^AstContext, symbol: Symbol, ok := tr
 	}
 
 	#partial switch &v in symbol.value {
+	case SymbolPackageValue:
+		if pkg, ok := indexer.index.collection.packages[symbol.pkg]; ok {
+			if symbol.doc == "" {
+				symbol.doc = strings.to_string(pkg.doc)
+			}
+			if symbol.comment == "" {
+				symbol.comment = strings.to_string(pkg.comment)
+			}
+		}
+		return symbol, true
 	case SymbolProcedureGroupValue:
 		if s, ok := resolve_function_overload(ast_context, v.group.derived.(^ast.Proc_Group)); ok {
 			if s.doc == "" {
diff --git a/src/server/check.odin b/src/server/check.odin
index e8400d3..78eb25f 100644
--- a/src/server/check.odin
+++ b/src/server/check.odin
@@ -1,6 +1,5 @@
 package server
 
-import "base:intrinsics"
 import "base:runtime"
 
 import "core:encoding/json"
@@ -11,11 +10,7 @@ import "core:os"
 import "core:path/filepath"
 import path "core:path/slashpath"
 import "core:slice"
-import "core:strconv"
 import "core:strings"
-import "core:sync"
-import "core:text/scanner"
-import "core:thread"
 
 import "src:common"
 
diff --git a/src/server/collector.odin b/src/server/collector.odin
index 708c96b..24fd0fa 100644
--- a/src/server/collector.odin
+++ b/src/server/collector.odin
@@ -1,6 +1,7 @@
 #+feature using-stmt
 package server
 
+import "core:fmt"
 import "core:mem"
 import "core:odin/ast"
 import "core:path/filepath"
@@ -38,6 +39,8 @@ SymbolPackage :: struct {
 	methods:            map[Method][dynamic]Symbol,
 	imports:            [dynamic]string, //Used for references to figure whether the package is even able to reference the symbol
 	proc_group_members: map[string]bool, // Tracks procedure names that are part of proc groups (used by fake methods)
+	doc:                strings.Builder,
+	comment:            strings.Builder,
 }
 
 get_index_unique_string :: proc {
@@ -477,6 +480,8 @@ get_or_create_package :: proc(collection: ^SymbolCollection, pkg_name: string) -
 		pkg.methods = make(map[Method][dynamic]Symbol, 100, collection.allocator)
 		pkg.objc_structs = make(map[string]ObjcStruct, 5, collection.allocator)
 		pkg.proc_group_members = make(map[string]bool, 10, collection.allocator)
+		pkg.doc = strings.builder_make(collection.allocator)
+		pkg.comment = strings.builder_make(collection.allocator)
 	}
 	return pkg
 }
@@ -662,6 +667,49 @@ collect_imports :: proc(collection: ^SymbolCollection, file: ast.File, directory
 
 }
 
+@(private = "file")
+get_symbol_package_name :: proc(
+	collection: ^SymbolCollection,
+	directory: string,
+	uri: string,
+	treat_as_builtin := false,
+) -> string {
+	if treat_as_builtin || strings.contains(uri, "builtin.odin") {
+		return "$builtin"
+	}
+
+	if strings.contains(uri, "intrinsics.odin") {
+		intrinsics_path := filepath.join(
+			elems = {common.config.collections["base"], "/intrinsics"},
+			allocator = context.temp_allocator,
+		)
+		intrinsics_path, _ = filepath.to_slash(intrinsics_path, context.temp_allocator)
+		return get_index_unique_string(collection, intrinsics_path)
+	}
+
+	return get_index_unique_string(collection, directory)
+}
+
+@(private = "file")
+get_package_decl_doc_comment :: proc(file: ast.File, allocator := context.temp_allocator) -> (string, string) {
+	if file.pkg_decl != nil {
+		docs := get_comment(file.pkg_decl.docs, allocator = allocator)
+		comment := get_comment(file.pkg_decl.comment, allocator = allocator)
+		return docs, comment
+	}
+	return "", ""
+}
+
+@(private = "file")
+write_doc_string :: proc(sb: ^strings.Builder, doc: string) {
+	if doc != "" {
+		if strings.builder_len(sb^) > 0 {
+			fmt.sbprintf(sb, "\n%s", doc)
+		} else {
+			strings.write_string(sb, doc)
+		}
+	}
+}
 
 collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: string) -> common.Error {
 	forward, _ := filepath.to_slash(file.fullpath, context.temp_allocator)
@@ -669,6 +717,12 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 	package_map := get_package_mapping(file, collection.config, directory)
 	exprs := collect_globals(file)
 
+	file_pkg_name := get_symbol_package_name(collection, directory, uri)
+	file_pkg := get_or_create_package(collection, file_pkg_name)
+	doc, comment := get_package_decl_doc_comment(file, collection.allocator)
+	write_doc_string(&file_pkg.doc, doc)
+	write_doc_string(&file_pkg.comment, comment)
+
 	for expr in exprs {
 		symbol: Symbol
 
@@ -694,18 +748,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 		}
 
 		// Compute pkg early so it's available inside the switch
-		if expr.builtin || strings.contains(uri, "builtin.odin") {
-			symbol.pkg = "$builtin"
-		} else if strings.contains(uri, "intrinsics.odin") {
-			intrinsics_path := filepath.join(
-				elems = {common.config.collections["base"], "/intrinsics"},
-				allocator = context.temp_allocator,
-			)
-			intrinsics_path, _ = filepath.to_slash(intrinsics_path, context.temp_allocator)
-			symbol.pkg = get_index_unique_string(collection, intrinsics_path)
-		} else {
-			symbol.pkg = get_index_unique_string(collection, directory)
-		}
+		symbol.pkg = get_symbol_package_name(collection, directory, uri, expr.builtin)
 
 		#partial switch v in col_expr.derived {
 		case ^ast.Matrix_Type:
@@ -899,17 +942,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 			symbol.flags |= {.Mutable}
 		}
 
-		pkg: ^SymbolPackage
-		ok: bool
-
-		if pkg, ok = &collection.packages[symbol.pkg]; !ok {
-			collection.packages[symbol.pkg] = {}
-			pkg = &collection.packages[symbol.pkg]
-			pkg.symbols = make(map[string]Symbol, 100, collection.allocator)
-			pkg.methods = make(map[Method][dynamic]Symbol, 100, collection.allocator)
-			pkg.objc_structs = make(map[string]ObjcStruct, 5, collection.allocator)
-			pkg.proc_group_members = make(map[string]bool, 10, collection.allocator)
-		}
+		pkg := get_or_create_package(collection, symbol.pkg)
 
 		if .ObjC in symbol.flags {
 			collect_objc(collection, expr.attributes, symbol)
@@ -942,19 +975,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri
 collect_fake_methods :: proc(collection: ^SymbolCollection, exprs: []GlobalExpr, directory: string, uri: string) {
 	for expr in exprs {
 		// Determine the package name (same logic as in collect_symbols)
-		pkg_name: string
-		if expr.builtin || strings.contains(uri, "builtin.odin") {
-			pkg_name = "$builtin"
-		} else if strings.contains(uri, "intrinsics.odin") {
-			intrinsics_path := filepath.join(
-				elems = {common.config.collections["base"], "/intrinsics"},
-				allocator = context.temp_allocator,
-			)
-			intrinsics_path, _ = filepath.to_slash(intrinsics_path, context.temp_allocator)
-			pkg_name = get_index_unique_string(collection, intrinsics_path)
-		} else {
-			pkg_name = get_index_unique_string(collection, directory)
-		}
+		pkg_name := get_symbol_package_name(collection, directory, uri, expr.builtin)
 
 		pkg, ok := &collection.packages[pkg_name]
 		if !ok {
diff --git a/src/server/completion.odin b/src/server/completion.odin
index 5bbbcf8..072d21b 100644
--- a/src/server/completion.odin
+++ b/src/server/completion.odin
@@ -1812,6 +1812,12 @@ get_identifier_completion :: proc(
 		symbol := Symbol {
 			name = pkg.base,
 			type = .Package,
+			pkg = pkg.name,
+			value = SymbolPackageValue{},
+		}
+		try_build_package(symbol.pkg)
+		if resolved, ok := resolve_symbol_return(ast_context, symbol); ok {
+			symbol = resolved
 		}
 
 		if score, ok := common.fuzzy_match(matcher, symbol.name); ok == 1 {
diff --git a/src/server/definition.odin b/src/server/definition.odin
index 869816c..7ff32d8 100644
--- a/src/server/definition.odin
+++ b/src/server/definition.odin
@@ -2,17 +2,9 @@ package server
 
 import "core:fmt"
 import "core:log"
-import "core:mem"
 import "core:odin/ast"
-import "core:odin/parser"
 import "core:odin/tokenizer"
-import "core:os"
 import "core:path/filepath"
-import path "core:path/slashpath"
-import "core:slice"
-import "core:sort"
-import "core:strconv"
-import "core:strings"
 
 import "src:common"
 
diff --git a/src/server/hover.odin b/src/server/hover.odin
index 9b42f7e..991ea1b 100644
--- a/src/server/hover.odin
+++ b/src/server/hover.odin
@@ -55,10 +55,45 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 		get_locals(document.ast, position_context.function, &ast_context, &position_context)
 	}
 
-	if position_context.import_stmt != nil {
+	if position_context.import_stmt != nil && position_in_node(position_context.import_stmt, position_context.position) {
+		for imp in document.imports {
+			if imp.original != position_context.import_stmt.fullpath {
+				continue
+			}
+
+			symbol := Symbol {
+				name  = imp.base,
+				type  = .Package,
+				pkg   = imp.name,
+				value = SymbolPackageValue{},
+			}
+			try_build_package(symbol.pkg)
+			if symbol, ok = resolve_symbol_return(&ast_context, symbol); ok {
+				hover.range = common.get_token_range(document.ast.pkg_decl, ast_context.file.src)
+				hover.contents = write_hover_content(&ast_context, symbol)
+				return hover, true, true
+			}
+		}
+
 		return {}, false, true
 	}
 
+	if document.ast.pkg_decl != nil && position_in_node(document.ast.pkg_decl, position_context.position) {
+		symbol := Symbol {
+			name  = document.ast.pkg_name,
+			type  = .Package,
+			pkg   = ast_context.document_package,
+			value = SymbolPackageValue{},
+		}
+		try_build_package(symbol.pkg)
+		if symbol, ok = resolve_symbol_return(&ast_context, symbol); ok {
+			hover.range = common.get_token_range(document.ast.pkg_decl, ast_context.file.src)
+			hover.contents = write_hover_content(&ast_context, symbol)
+			return hover, true, true
+		}
+
+	}
+
 	if position_context.type_cast != nil &&
 	   !position_in_node(position_context.type_cast.type, position_context.position) &&
 	   !position_in_node(position_context.type_cast.expr, position_context.position) { 	// check that we're actually on the 'cast' word
diff --git a/tests/completions_test.odin b/tests/completions_test.odin
index 56553c3..bf05f6a 100644
--- a/tests/completions_test.odin
+++ b/tests/completions_test.odin
@@ -5500,3 +5500,31 @@ ast_completion_fake_method_proc_group_single_arg_cursor_position :: proc(t: ^tes
 	// The proc group 'negate' should have cursor AFTER parentheses since no additional args
 	test.expect_completion_edit_text(t, &source, ".", "negate", "methods.negate(n)$0")
 }
+
+@(test)
+ast_completion_package_docs :: proc(t: ^testing.T) {
+	packages := make([dynamic]test.Package, context.temp_allocator)
+
+	append(
+		&packages,
+		test.Package {
+			pkg = "my_package",
+			source = `// Package docs
+		package my_package
+		Foo :: struct{}
+		`,
+		},
+	)
+
+	source := test.Source {
+		main     = `package test
+		import "my_package"
+		main :: proc() {
+			my_pack{*}
+		}
+		`,
+		packages = packages[:],
+	}
+
+	test.expect_completion_docs(t, &source, "", {"my_package: package\n---\nPackage docs"})
+}
diff --git a/tests/hover_test.odin b/tests/hover_test.odin
index 35c2c4f..74cf0d0 100644
--- a/tests/hover_test.odin
+++ b/tests/hover_test.odin
@@ -6105,6 +6105,57 @@ ast_hover_parapoly_overloaded_proc_with_bitfield :: proc(t: ^testing.T) {
 	}
 	test.expect_hover(t, &source, "test.entry: test.Entry(int, SmallHandle)")
 }
+
+@(test)
+ast_hover_package_docs :: proc(t: ^testing.T) {
+	packages := make([dynamic]test.Package, context.temp_allocator)
+
+	append(
+		&packages,
+		test.Package {
+			pkg = "my_package",
+			source = `// Package docs
+		package my_package
+		Foo :: struct{}
+		`,
+		},
+	)
+	source := test.Source {
+		main     = `package test
+		import "my_package"
+		main :: proc() {
+			foo := my_packa{*}ge.Foo{}
+		}
+		`,
+		packages = packages[:],
+	}
+
+	test.expect_hover(t, &source, "my_package: package\n---\nPackage docs")
+}
+
+@(test)
+ast_hover_import_path_package_docs :: proc(t: ^testing.T) {
+	packages := make([dynamic]test.Package, context.temp_allocator)
+
+	append(
+		&packages,
+		test.Package {
+			pkg = "my_package",
+			source = `// Package docs
+		package my_package
+		`,
+		},
+	)
+	source := test.Source {
+		main     = `package test
+		import "my_packa{*}ge"
+		`,
+		packages = packages[:],
+	}
+
+	test.expect_hover(t, &source, "my_package: package\n---\nPackage docs")
+}
+
 /*
 
 Waiting for odin fix