Merge pull request #1040 from DanielGavin/remove-unused-imports

Add new code action: remove unused imports

10 files changed, 328 insertions, 152 deletions

diff --git a/src/common/config.odin b/src/common/config.odin
index c0ddb0f..b44623a 100644
--- a/src/common/config.odin
+++ b/src/common/config.odin
@@ -10,41 +10,42 @@ ConfigProfile :: struct {
 }
 
 Config :: struct {
-	workspace_folders:                 [dynamic]WorkspaceFolder,
-	completion_support_md:             bool,
-	hover_support_md:                  bool,
-	signature_offset_support:          bool,
-	collections:                       map[string]string,
-	running:                           bool,
-	verbose:                           bool,
-	enable_format:                     bool,
-	enable_hover:                      bool,
-	enable_document_symbols:           bool,
-	enable_semantic_tokens:            bool,
-	enable_inlay_hints_params:         bool,
-	enable_inlay_hints_default_params: bool,
+	workspace_folders:                  [dynamic]WorkspaceFolder,
+	completion_support_md:              bool,
+	hover_support_md:                   bool,
+	signature_offset_support:           bool,
+	collections:                        map[string]string,
+	running:                            bool,
+	verbose:                            bool,
+	enable_format:                      bool,
+	enable_hover:                       bool,
+	enable_document_symbols:            bool,
+	enable_semantic_tokens:             bool,
+	enable_unused_imports_reporting:    bool,
+	enable_inlay_hints_params:          bool,
+	enable_inlay_hints_default_params:  bool,
 	enable_inlay_hints_implicit_return: bool,
-	enable_procedure_context:          bool,
-	enable_snippets:                   bool,
-	enable_references:                 bool,
-	enable_label_details:              bool,
-	enable_std_references:             bool,
-	enable_import_fixer:               bool,
-	enable_fake_method:                bool,
-	enable_procedure_snippet:          bool,
-	enable_checker_only_saved:         bool,
-	enable_auto_import:                bool,
-	enable_completion_matching:        bool,
-	enable_document_links:             bool,
-	disable_parser_errors:             bool,
-	thread_count:                      int,
-	file_log:                          bool,
-	odin_command:                      string,
-	odin_root_override:                string,
-	checker_args:                      string,
-	checker_targets:                   []string,
-	client_name:                       string,
-	profile:                           ConfigProfile,
+	enable_procedure_context:           bool,
+	enable_snippets:                    bool,
+	enable_references:                  bool,
+	enable_label_details:               bool,
+	enable_std_references:              bool,
+	enable_import_fixer:                bool,
+	enable_fake_method:                 bool,
+	enable_procedure_snippet:           bool,
+	enable_checker_only_saved:          bool,
+	enable_auto_import:                 bool,
+	enable_completion_matching:         bool,
+	enable_document_links:              bool,
+	disable_parser_errors:              bool,
+	thread_count:                       int,
+	file_log:                           bool,
+	odin_command:                       string,
+	odin_root_override:                 string,
+	checker_args:                       string,
+	checker_targets:                    []string,
+	client_name:                        string,
+	profile:                            ConfigProfile,
 }
 
 config: Config
diff --git a/src/common/position.odin b/src/common/position.odin
index e5dc2bd..4d01f58 100644
--- a/src/common/position.odin
+++ b/src/common/position.odin
@@ -133,6 +133,20 @@ get_token_range :: proc(node: ast.Node, document_text: string) -> (range: Range)
 	return
 }
 
+get_last_column :: proc(line: int, document_text: []u8) -> (int, bool) {
+	line_count := 0
+	index := 1
+	last := document_text[0]
+
+	if !get_index_at_line(&index, &line_count, &last,  document_text[:], line) {
+		return {}, false
+	}
+
+	column := get_character_offset_u8_to_u16(100000, document_text[index:])
+
+	return column, true
+}
+
 get_absolute_range :: proc(range: Range, document_text: []u8) -> (AbsoluteRange, bool) {
 	absolute: AbsoluteRange
 
diff --git a/src/server/action.odin b/src/server/action.odin
index 0c3c74a..d1608b5 100644
--- a/src/server/action.odin
+++ b/src/server/action.odin
@@ -67,11 +67,63 @@ get_code_actions :: proc(document: ^Document, range: common.Range, config: ^comm
 		if selector, ok := position_context.selector_expr.derived.(^ast.Selector_Expr); ok {
 			add_missing_imports(&ast_context, selector, strings.clone(document.uri.uri), config, &actions)
 		}
+	} else if position_context.import_stmt != nil {
+		remove_unused_imports(document, strings.clone(document.uri.uri), config, &actions)
 	}
 
 	return actions[:], true
 }
 
+remove_unused_imports :: proc(
+	document: ^Document,
+	uri: string,
+	config: ^common.Config,
+	actions: ^[dynamic]CodeAction,
+) {
+	unused_imports := find_unused_imports(document, context.temp_allocator)
+
+	if len(unused_imports) == 0 {
+		return
+	}
+
+	textEdits := make([dynamic]TextEdit, context.temp_allocator)
+
+	for imp in unused_imports {
+		range := common.get_token_range(imp.import_decl, document.ast.src)
+
+		import_edit := TextEdit {
+			range   = range,
+			newText = "",
+		}
+
+		if (range.start.line != 1) {
+			if column, ok := common.get_last_column(import_edit.range.start.line - 1, document.text); ok {
+				import_edit.range.start.line -= 1
+				import_edit.range.start.character = column
+			}
+
+		}
+
+
+		append(&textEdits, import_edit)
+	}
+
+	workspaceEdit: WorkspaceEdit
+	workspaceEdit.changes = make(map[string][]TextEdit, 0, context.temp_allocator)
+	workspaceEdit.changes[uri] = textEdits[:]
+
+	append(
+		actions,
+		CodeAction {
+			kind = "refactor.rewrite",
+			isPreferred = true,
+			title = fmt.tprint("remove unused imports"),
+			edit = workspaceEdit,
+		},
+	)
+
+}
+
 add_missing_imports :: proc(
 	ast_context: ^AstContext,
 	selector: ^ast.Selector_Expr,
diff --git a/src/server/check.odin b/src/server/check.odin
index b8f0c7c..b53d7e2 100644
--- a/src/server/check.odin
+++ b/src/server/check.odin
@@ -19,9 +19,6 @@ import "core:thread"
 
 import "src:common"
 
-//Store uris we have reported on since last save. We use this to clear them on next save.
-uris_reported: [dynamic]string
-
 Json_Error :: struct {
 	type: string,
 	pos:  Json_Type_Error,
@@ -68,7 +65,31 @@ fallback_find_odin_directories :: proc(config: ^common.Config) -> []string {
 	return data[:]
 }
 
-check :: proc(paths: []string, uri: common.Uri, writer: ^Writer, config: ^common.Config) {
+check_unused_imports :: proc(document: ^Document, config: ^common.Config) {
+	if !config.enable_unused_imports_reporting {
+		return
+	}
+
+	unused_imports := find_unused_imports(document, context.temp_allocator)
+
+	remove_diagnostics(.Unused, document.uri.uri)
+
+	for imp in unused_imports {
+		add_diagnostics(
+			.Unused,
+			document.uri.uri,
+			Diagnostic {
+				range = common.get_token_range(imp.import_decl, document.ast.src),
+				severity = DiagnosticSeverity.Hint,
+				code = "Unused",
+				message = "unused import",
+				tags = {.Unnecessary},
+			},
+		)
+	}
+}
+
+check :: proc(paths: []string, uri: common.Uri, config: ^common.Config) {
 	paths := paths
 
 	if len(paths) == 0 {
@@ -138,6 +159,8 @@ check :: proc(paths: []string, uri: common.Uri, writer: ^Writer, config: ^common
 			log.errorf("Failed to unmarshal check results: %v, %v", res, string(buffer))
 		}
 
+		clear_diagnostics(.Check)
+
 		for error in json_errors.errors {
 			if len(error.msgs) == 0 {
 				break
@@ -149,12 +172,11 @@ check :: proc(paths: []string, uri: common.Uri, writer: ^Writer, config: ^common
 				continue
 			}
 
-			if error.pos.file not_in errors {
-				errors[error.pos.file] = make([dynamic]Diagnostic, context.temp_allocator)
-			}
+			uri := common.create_uri(error.pos.file, context.temp_allocator)
 
-			append(
-				&errors[error.pos.file],
+			add_diagnostics(
+				.Check,
+				uri.uri,
 				Diagnostic {
 					code = "checker",
 					severity = .Error,
@@ -168,51 +190,4 @@ check :: proc(paths: []string, uri: common.Uri, writer: ^Writer, config: ^common
 			)
 		}
 	}
-
-	for uri in uris_reported {
-		params := NotificationPublishDiagnosticsParams {
-			uri         = uri,
-			diagnostics = {},
-		}
-
-		notification := Notification {
-			jsonrpc = "2.0",
-			method  = "textDocument/publishDiagnostics",
-			params  = params,
-		}
-
-		if writer != nil {
-			send_notification(notification, writer)
-		}
-
-		delete(uri)
-	}
-
-	clear(&uris_reported)
-
-	for k, v in errors {
-		uri := common.create_uri(k, context.temp_allocator)
-
-		//Find the unique diagnostics, since some poor profile settings make the checker check the same file multiple times
-		unique := slice.unique(v[:])
-
-		params := NotificationPublishDiagnosticsParams {
-			uri         = uri.uri,
-			diagnostics = unique,
-		}
-
-		notifaction := Notification {
-			jsonrpc = "2.0",
-			method  = "textDocument/publishDiagnostics",
-			params  = params,
-		}
-
-		append(&uris_reported, strings.clone(uri.uri))
-
-		if writer != nil {
-			send_notification(notifaction, writer)
-		}
-	}
-
-
 }
diff --git a/src/server/diagnostics.odin b/src/server/diagnostics.odin
new file mode 100644
index 0000000..411ffbc
--- /dev/null
+++ b/src/server/diagnostics.odin
@@ -0,0 +1,126 @@
+package server
+
+import "core:log"
+import "core:slice"
+import "core:strings"
+import "src:common"
+
+DiagnosticType :: enum {
+	Syntax,
+	Unused,
+	Check,
+}
+
+diagnostics: [DiagnosticType]map[string][dynamic]Diagnostic
+
+add_diagnostics :: proc(type: DiagnosticType, uri: string, diagnostic: Diagnostic) {
+	diagnostic_type := &diagnostics[type]
+
+	if diagnostic_type == nil {
+		log.errorf("Diagnostic type did not exist: %v", type)
+		return
+	}
+
+	uri := uri
+
+	when ODIN_OS == .Windows {
+		uri = strings.to_lower(uri, context.temp_allocator)
+	}
+
+	diagnostic_array := &diagnostic_type[uri]
+
+	if diagnostic_array == nil {
+		diagnostic_type[strings.clone(uri)] = make([dynamic]Diagnostic)
+		diagnostic_array = &diagnostic_type[uri]
+	}
+
+    diagnostic := diagnostic
+
+    diagnostic.message = strings.clone(diagnostic.message)
+    diagnostic.code = strings.clone(diagnostic.code)
+
+    append(diagnostic_array, diagnostic)
+}
+
+remove_diagnostics :: proc(type: DiagnosticType, uri: string) {
+	diagnostic_type := &diagnostics[type]
+
+	if diagnostic_type == nil {
+		log.errorf("Diagnostic type did not exist: %v", type)
+		return
+	}
+
+	uri := uri
+
+	when ODIN_OS == .Windows {
+		uri = strings.to_lower(uri, context.temp_allocator)
+	}
+
+	diagnostic_array := &diagnostic_type[uri]
+
+	if diagnostic_array == nil {
+		return
+	}
+
+	for diagnostic in diagnostic_array {
+		delete(diagnostic.message)
+		delete(diagnostic.code)
+	}
+
+	clear(diagnostic_array)
+}
+
+clear_diagnostics :: proc(type: DiagnosticType) {
+	diagnostic_type := &diagnostics[type]
+
+	if diagnostic_type == nil {
+		log.errorf("Diagnostic type did not exist: %v", type)
+		return
+	}
+
+	for _, &diagnostic_array in diagnostic_type {
+		for diagnostic in diagnostic_array {
+			delete(diagnostic.message)
+			delete(diagnostic.code)
+		}
+		clear(&diagnostic_array)
+	}
+}
+
+push_diagnostics :: proc(writer: ^Writer) {
+	merged_diagnostics := make(map[string][dynamic]Diagnostic, context.temp_allocator)
+
+	for diagnostic_type in diagnostics {
+		for k, v in diagnostic_type {
+			diagnostic_array := &merged_diagnostics[k]
+
+			if diagnostic_array == nil {
+				merged_diagnostics[k] = make([dynamic]Diagnostic, context.temp_allocator)
+				diagnostic_array = &merged_diagnostics[k]
+			}
+
+			append(diagnostic_array, ..v[:])
+		}
+	}
+
+	for k, v in merged_diagnostics {
+		//Find the unique diagnostics, since some poor profile settings make the checker check the same file multiple times
+		unique := slice.unique(v[:])
+
+		params := NotificationPublishDiagnosticsParams {
+			uri         = k,
+			diagnostics = unique,
+		}
+
+		notifaction := Notification {
+			jsonrpc = "2.0",
+			method  = "textDocument/publishDiagnostics",
+			params  = params,
+		}
+
+		if writer != nil {
+			send_notification(notifaction, writer)
+		}
+	}
+
+}
diff --git a/src/server/documents.odin b/src/server/documents.odin
index 15e7ad6..a23fdf2 100644
--- a/src/server/documents.odin
+++ b/src/server/documents.odin
@@ -30,6 +30,7 @@ Package :: struct {
 	base_original: string,
 	original:      string,
 	range:         common.Range,
+	import_decl:   ^ast.Import_Decl,
 }
 
 Document :: struct {
@@ -158,8 +159,6 @@ document_open :: proc(uri_string: string, text: string, config: ^common.Config,
 		document_storage.documents[strings.clone(uri.path)] = document
 	}
 
-	delete(uri_string)
-
 	return .None
 }
 
@@ -319,52 +318,29 @@ document_refresh :: proc(document: ^Document, config: ^common.Config, writer: ^W
 		return .None
 	}
 
-	if writer != nil && len(errors) > 0 && !config.disable_parser_errors {
-		document.diagnosed_errors = true
+	remove_diagnostics(.Syntax, document.uri.uri)
+	remove_diagnostics(.Check, document.uri.uri)
 
-		params := NotificationPublishDiagnosticsParams {
-			uri         = document.uri.uri,
-			diagnostics = make([]Diagnostic, len(errors), context.temp_allocator),
-		}
+	if writer != nil && !config.disable_parser_errors {
+		document.diagnosed_errors = true
 
 		for error, i in errors {
-			params.diagnostics[i] = Diagnostic {
-				range = common.Range {
-					start = common.Position{line = error.line - 1, character = 0},
-					end = common.Position{line = error.line, character = 0},
+			add_diagnostics(
+				.Syntax,
+				document.uri.uri,
+				Diagnostic {
+					range = common.Range {
+						start = common.Position{line = error.line - 1, character = 0},
+						end = common.Position{line = error.line, character = 0},
+					},
+					severity = DiagnosticSeverity.Error,
+					code = "Syntax",
+					message = error.message,
 				},
-				severity = DiagnosticSeverity.Error,
-				code = "Syntax",
-				message = error.message,
-			}
-		}
-
-		notifaction := Notification {
-			jsonrpc = "2.0",
-			method  = "textDocument/publishDiagnostics",
-			params  = params,
+			)
 		}
 
-		send_notification(notifaction, writer)
-	}
-
-	if writer != nil && len(errors) == 0 {
-		//send empty diagnosis to remove the clients errors
-		if document.diagnosed_errors {
-
-			notifaction := Notification {
-				jsonrpc = "2.0",
-				method = "textDocument/publishDiagnostics",
-				params = NotificationPublishDiagnosticsParams {
-					uri = document.uri.uri,
-					diagnostics = make([]Diagnostic, len(errors), context.temp_allocator),
-				},
-			}
-
-			document.diagnosed_errors = false
-
-			send_notification(notifaction, writer)
-		}
+		push_diagnostics(writer)
 	}
 
 	return .None
@@ -450,6 +426,7 @@ parse_imports :: proc(document: ^Document, config: ^common.Config) {
 			import_.original = imp.fullpath
 			import_.name = strings.clone(path.join(elems = {dir, p}, allocator = context.temp_allocator))
 			import_.range = range
+			import_.import_decl = imp
 
 			if imp.name.text != "" {
 				import_.base = imp.name.text
@@ -473,6 +450,7 @@ parse_imports :: proc(document: ^Document, config: ^common.Config) {
 			)
 			import_.name = path.clean(import_.name)
 			import_.range = range
+			import_.import_decl = imp
 
 			if imp.name.text != "" {
 				import_.base = imp.name.text
@@ -499,18 +477,12 @@ get_import_range :: proc(imp: ^ast.Import_Decl, src: string) -> common.Range {
 		start := common.token_pos_to_position(imp.name.pos, src)
 		end := start
 		end.character += len(imp.name.text)
-		return {
-			start = start,
-			end = end,
-		}
+		return {start = start, end = end}
 	}
 
 	start := common.token_pos_to_position(imp.relpath.pos, src)
 	end := start
 	text_len := len(imp.relpath.text)
 	end.character += text_len
-	return {
-		start = start,
-		end = end,
-	}
+	return {start = start, end = end}
 }
diff --git a/src/server/format.odin b/src/server/format.odin
index d981b9c..9f30c49 100644
--- a/src/server/format.odin
+++ b/src/server/format.odin
@@ -29,10 +29,6 @@ get_complete_format :: proc(document: ^Document, config: ^common.Config) -> ([]T
 		return {}, true
 	}
 
-	if config.enable_import_fixer {
-		fix_imports(document)
-	}
-
 	style := format.find_config_file_or_default(filepath.dir(document.fullpath, context.temp_allocator))
 	prnt := printer.make_printer(style, context.temp_allocator)
 
diff --git a/src/server/imports.odin b/src/server/imports.odin
index cd2271d..498af33 100644
--- a/src/server/imports.odin
+++ b/src/server/imports.odin
@@ -1,17 +1,35 @@
 package server
 
+import "core:log"
 import "core:mem"
 import "core:odin/ast"
 
+import "base:runtime"
 
-fix_imports :: proc(document: ^Document) {
-	arena: mem.Arena
-	mem.arena_init(&arena, make([]byte, mem.Megabyte * 25))
-	defer delete(arena.data)
+find_unused_imports :: proc(document: ^Document, allocator := context.temp_allocator) -> []Package {
+	arena: runtime.Arena
 
-	context.allocator = mem.arena_allocator(&arena)
+	_ = runtime.arena_init(&arena, mem.Megabyte * 40, runtime.default_allocator())
 
-	symbols_and_nodes := resolve_entire_file(document, .None)
+	defer runtime.arena_destroy(&arena)
 
+	context.allocator = runtime.arena_allocator(&arena)
 
+	symbols_and_nodes := resolve_entire_file(document)
+
+	pkgs := make(map[string]struct{}, context.temp_allocator)
+
+	for _, v in symbols_and_nodes {
+		pkgs[v.symbol.pkg] = {}
+	}
+
+	unused := make([dynamic]Package, allocator)
+
+	for imp in document.imports {
+		if imp.name not_in pkgs {
+			append(&unused, imp)
+		}
+	}
+
+	return unused[:]
 }
diff --git a/src/server/requests.odin b/src/server/requests.odin
index 511832a..899d060 100644
--- a/src/server/requests.odin
+++ b/src/server/requests.odin
@@ -362,6 +362,7 @@ read_ols_initialize_options :: proc(config: ^common.Config, ols_config: OlsConfi
 	config.enable_format = ols_config.enable_format.(bool) or_else config.enable_format
 	config.enable_hover = ols_config.enable_hover.(bool) or_else config.enable_hover
 	config.enable_semantic_tokens = ols_config.enable_semantic_tokens.(bool) or_else config.enable_semantic_tokens
+	config.enable_unused_imports_reporting = ols_config.enable_unused_imports_reporting.(bool) or_else config.enable_unused_imports_reporting
 	config.enable_procedure_context =
 		ols_config.enable_procedure_context.(bool) or_else config.enable_procedure_context
 	config.enable_snippets = ols_config.enable_snippets.(bool) or_else config.enable_snippets
@@ -589,7 +590,6 @@ request_initialize :: proc(
 	initialize_params: RequestInitializeParams
 
 	if err := unmarshal(params, initialize_params, context.temp_allocator); err != nil {
-		log.error("Here?", err, params)
 		return .ParseError
 	}
 
@@ -615,6 +615,7 @@ request_initialize :: proc(
 	config.enable_format = true
 	config.enable_hover = true
 	config.enable_semantic_tokens = false
+	config.enable_unused_imports_reporting = true
 	config.enable_procedure_context = false
 	config.enable_snippets = false
 	config.enable_references = true
@@ -1035,10 +1036,18 @@ notification_did_open :: proc(
 		return .ParseError
 	}
 
+	defer delete(open_params.textDocument.uri)
+
 	if n := document_open(open_params.textDocument.uri, open_params.textDocument.text, config, writer); n != .None {
 		return .InternalError
 	}
 
+	document := document_get(open_params.textDocument.uri)
+
+	check_unused_imports(document, config)
+
+	push_diagnostics(writer)
+
 	return .None
 }
 
@@ -1133,7 +1142,13 @@ notification_did_save :: proc(
 
 	corrected_uri := common.create_uri(fullpath, context.temp_allocator)
 
-	check(config.profile.checker_path[:], corrected_uri, writer, config)
+	check(config.profile.checker_path[:], corrected_uri, config)
+
+	document := document_get(save_params.textDocument.uri)
+
+	check_unused_imports(document, config)
+
+	push_diagnostics(writer)
 
 	return .None
 }
diff --git a/src/server/types.odin b/src/server/types.odin
index 269cc3f..e0a2667 100644
--- a/src/server/types.odin
+++ b/src/server/types.odin
@@ -275,11 +275,17 @@ DiagnosticSeverity :: enum {
 	Hint        = 4,
 }
 
+ DiagnosticTag :: enum int {
+	Unnecessary = 1,
+	Deprecated  = 2,
+ }
+
 Diagnostic :: struct {
 	range:    common.Range,
 	severity: DiagnosticSeverity,
 	code:     string,
 	message:  string,
+	tags:     [1]DiagnosticTag,
 }
 
 DidOpenTextDocumentParams :: struct {
@@ -418,6 +424,7 @@ OlsConfig :: struct {
 	enable_inlay_hints_default_params: Maybe(bool),
 	enable_inlay_hints_implicit_return: Maybe(bool),
 	enable_semantic_tokens:            Maybe(bool),
+	enable_unused_imports_reporting:   Maybe(bool),
 	enable_procedure_context:          Maybe(bool),
 	enable_snippets:                   Maybe(bool),
 	enable_procedure_snippet:          Maybe(bool),