From aab9ca18f24d4cd17cc3bc1cce026e810ad66a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20H=C3=A4hne?= Date: Tue, 30 Mar 2021 10:33:21 +0200 Subject: [PATCH] Fix some crashes & find all @imports If there are parse errors, an AST can contain uninitialized nodes. Walking the tree in this case can lead to horribly nasty crashes. --- src/analysis.zig | 108 ++++++++++++++++++---------------------- src/document_store.zig | 11 ++-- src/main.zig | 2 + src/references.zig | 3 ++ src/semantic_tokens.zig | 3 ++ 5 files changed, 64 insertions(+), 63 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index b060848..99fa530 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -222,6 +222,8 @@ pub fn isPascalCase(name: []const u8) bool { pub fn getDeclNameToken(tree: ast.Tree, node: ast.Node.Index) ?ast.TokenIndex { const tags = tree.nodes.items(.tag); const main_token = tree.nodes.items(.main_token)[node]; + if (tree.errors.len > 0) + return null; return switch (tags[node]) { // regular declaration names. + 1 to mut token because name comes after 'const'/'var' .local_var_decl => tree.localVarDecl(node).ast.mut_token + 1, @@ -346,6 +348,8 @@ pub fn resolveVarDeclAlias(store: *DocumentStore, arena: *std.heap.ArenaAllocato const token_tags = tree.tokens.items(.tag); const main_tokes = tree.nodes.items(.main_token); const node_tags = tree.nodes.items(.tag); + if (tree.errors.len > 0) + return null; if (varDecl(handle.tree, decl)) |var_decl| { if (var_decl.ast.init_node == 0) return null; @@ -1182,64 +1186,29 @@ pub fn resolveTypeOfNode(store: *DocumentStore, arena: *std.heap.ArenaAllocator, return resolveTypeOfNodeInternal(store, arena, node_handle, &bound_type_params); } -fn maybeCollectImport(tree: ast.Tree, builtin_call: ast.Node.Index, arr: *std.ArrayList([]const u8)) !void { - const tags = tree.nodes.items(.tag); - const datas = tree.nodes.items(.data); - - const builtin_tag = tags[builtin_call]; - const data = datas[builtin_call]; - - std.debug.assert(isBuiltinCall(tree, builtin_call)); - if (!std.mem.eql(u8, tree.tokenSlice(builtin_call), "@import")) return; - - const params = switch (builtin_tag) { - .builtin_call, .builtin_call_comma => tree.extra_data[data.lhs..data.rhs], - .builtin_call_two, .builtin_call_two_comma => if (data.lhs == 0) - &[_]ast.Node.Index{} - else if (data.rhs == 0) - &[_]ast.Node.Index{data.lhs} - else - &[_]ast.Node.Index{ data.lhs, data.rhs }, - else => unreachable, - }; - if (params.len != 1) return; - - if (tags[params[0]] != .string_literal) return; - - const import_str = tree.tokenSlice(tree.nodes.items(.main_token)[params[0]]); - try arr.append(import_str[1 .. import_str.len - 1]); -} - /// Collects all imports we can find into a slice of import paths (without quotes). -/// The import paths are valid as long as the tree is. pub fn collectImports(import_arr: *std.ArrayList([]const u8), tree: ast.Tree) !void { - // TODO: Currently only detects `const smth = @import("string literal")<.SomeThing>;` - const tags = tree.nodes.items(.tag); - for (tree.rootDecls()) |decl_idx| { - const var_decl_maybe: ?ast.full.VarDecl = switch (tags[decl_idx]) { - .global_var_decl => tree.globalVarDecl(decl_idx), - .local_var_decl => tree.localVarDecl(decl_idx), - .simple_var_decl => tree.simpleVarDecl(decl_idx), - else => null, - }; - const var_decl = var_decl_maybe orelse continue; - if (var_decl.ast.init_node == 0) continue; + const tags = tree.tokens.items(.tag); - const init_node = var_decl.ast.init_node; - const init_node_tag = tags[init_node]; - switch (init_node_tag) { - .builtin_call, - .builtin_call_comma, - .builtin_call_two, - .builtin_call_two_comma, - => try maybeCollectImport(tree, init_node, import_arr), - .field_access => { - const lhs = tree.nodes.items(.data)[init_node].lhs; - if (isBuiltinCall(tree, lhs)) { - try maybeCollectImport(tree, lhs, import_arr); - } - }, - else => {}, + while (i < tags.len) : (i += 1) { + if (tags[i] != .builtin) + continue; + const text = tree.tokenSlice(i); + log.debug("Found {}", .{ text }); + + if (std.mem.eql(u8, text, "@import")) { + if (i + 3 >= tags.len) + break; + if (tags[i + 1] != .l_paren) + continue; + if (tags[i + 2] != .string_literal) + continue; + if (tags[i + 3] != .r_paren) + continue; + + + const str = tree.tokenSlice(i + 2); + try import_arr.append(str[1..str.len-1]); } } } @@ -1265,6 +1234,8 @@ pub fn getFieldAccessType( .node = undefined, .handle = handle, }); + if (handle.tree.errors > 0) + return null; // TODO Actually bind params here when calling functions instead of just skipping args. var bound_type_params = BoundTypeParams.init(&arena.allocator); @@ -1910,6 +1881,8 @@ fn getDocumentSymbolsInternal(allocator: *std.mem.Allocator, tree: ast.Tree, nod pub fn getDocumentSymbols(allocator: *std.mem.Allocator, tree: ast.Tree, encoding: offsets.Encoding) ![]types.DocumentSymbol { var symbols = try std.ArrayList(types.DocumentSymbol).initCapacity(allocator, tree.rootDecls().len); + if (tree.errors.len > 0) + return 0; var context = GetDocumentSymbolsContext{ .symbols = &symbols, @@ -2467,6 +2440,12 @@ pub const DocumentScope = struct { error_completions: CompletionSet, enum_completions: CompletionSet, + pub const none = DocumentScope{ + .scopes = &[0]Scope{}, + .error_completions = CompletionSet{}, + .enum_completions = CompletionSet{}, + }; + pub fn debugPrint(self: DocumentScope) void { for (self.scopes) |scope| { log.debug( @@ -2530,6 +2509,9 @@ pub fn makeDocumentScope(allocator: *std.mem.Allocator, tree: ast.Tree) !Documen var error_completions = CompletionSet{}; var enum_completions = CompletionSet{}; + if (tree.errors.len > 0) + return DocumentScope.none; + errdefer { scopes.deinit(allocator); for (error_completions.entries.items) |entry| { @@ -2748,6 +2730,7 @@ fn makeScopeInternal( => |fn_tag| { var buf: [1]ast.Node.Index = undefined; const func = fnProto(tree, node_idx, &buf).?; + // log.debug("Alive 3.1", .{}); (try scopes.addOne(allocator)).* = .{ .range = nodeSourceRange(tree, node_idx), @@ -2781,7 +2764,10 @@ fn makeScopeInternal( param.type_expr, ); } - + const a = data[node_idx]; + const left = data[a.lhs]; + const right = data[a.rhs]; + // log.debug("Alive 3.2 - {}- {}- {}-{} {}- {}-{}", .{tags[node_idx], tags[a.lhs], tags[left.lhs], tags[left.rhs], tags[a.rhs], tags[right.lhs], tags[right.rhs]}); // Visit the return type try makeScopeInternal( allocator, @@ -2795,6 +2781,7 @@ fn makeScopeInternal( else data[node_idx].rhs, ); + log.debug("Alive 3.3", .{}); // Visit the function body if (fn_tag == .fn_decl) { try makeScopeInternal( @@ -3256,7 +3243,7 @@ fn makeScopeInternal( => { const field = containerField(tree, node_idx).?; - if (field.ast.type_expr != 0) + if (field.ast.type_expr != 0) { try makeScopeInternal( allocator, scopes, @@ -3265,7 +3252,8 @@ fn makeScopeInternal( tree, field.ast.type_expr, ); - if (field.ast.align_expr != 0) + } + if (field.ast.align_expr != 0) { try makeScopeInternal( allocator, scopes, @@ -3274,7 +3262,8 @@ fn makeScopeInternal( tree, field.ast.align_expr, ); - if (field.ast.value_expr != 0) + } + if (field.ast.value_expr != 0) { try makeScopeInternal( allocator, scopes, @@ -3283,6 +3272,7 @@ fn makeScopeInternal( tree, field.ast.value_expr, ); + } }, .builtin_call, .builtin_call_comma, diff --git a/src/document_store.zig b/src/document_store.zig index c67cbba..2560a0c 100644 --- a/src/document_store.zig +++ b/src/document_store.zig @@ -422,13 +422,16 @@ pub fn applyChanges( if (change.Object.get("range")) |range| { std.debug.assert(document.text.ptr == document.mem.ptr); + // TODO: add tests and validate the JSON + const start_obj = range.Object.get("start").?.Object; const start_pos = types.Position{ - .line = range.Object.get("start").?.Object.get("line").?.Integer, - .character = range.Object.get("start").?.Object.get("character").?.Integer, + .line = start_obj.get("line").?.Integer, + .character = start_obj.get("character").?.Integer, }; + const end_obj = range.Object.get("end").?.Object; const end_pos = types.Position{ - .line = range.Object.get("end").?.Object.get("line").?.Integer, - .character = range.Object.get("end").?.Object.get("character").?.Integer, + .line = end_obj.get("line").?.Integer, + .character = end_obj.get("character").?.Integer, }; const change_text = change.Object.get("text").?.String; diff --git a/src/main.zig b/src/main.zig index ef38310..bf4ff52 100644 --- a/src/main.zig +++ b/src/main.zig @@ -356,6 +356,8 @@ fn nodeToCompletion( const node_tags = tree.nodes.items(.tag); const datas = tree.nodes.items(.data); const token_tags = tree.tokens.items(.tag); + if (tree.errors.len > 0) + return; const doc_kind: types.MarkupContent.Kind = if (client_capabilities.completion_doc_supports_md) .Markdown diff --git a/src/references.zig b/src/references.zig index a5c2988..d301bda 100644 --- a/src/references.zig +++ b/src/references.zig @@ -85,6 +85,9 @@ fn symbolReferencesInternal( const main_tokens = tree.nodes.items(.main_token); const starts = tree.tokens.items(.start); + if (tree.errors.len > 0) + return; + switch (node_tags[node]) { .block, .block_semicolon, .block_two, .block_two_semicolon => { const statements: []const ast.Node.Index = switch (node_tags[node]) { diff --git a/src/semantic_tokens.zig b/src/semantic_tokens.zig index 72cecd0..7e1fc99 100644 --- a/src/semantic_tokens.zig +++ b/src/semantic_tokens.zig @@ -1145,6 +1145,9 @@ fn writeNodeTokens( // TODO Range version, edit version. pub fn writeAllSemanticTokens(arena: *std.heap.ArenaAllocator, store: *DocumentStore, handle: *DocumentStore.Handle, encoding: offsets.Encoding) ![]u32 { var builder = Builder.init(arena.child_allocator, handle, encoding); + if (handle.tree.errors.len > 0) { + return builder.toOwnedSlice(); + } // reverse the ast from the root declarations var gap_highlighter = GapHighlighter.init(&builder, 0);