-
Notifications
You must be signed in to change notification settings - Fork 17.5k
[clangd] Add InsertReplaceEdit for code completion #187623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
45639ed
1f22f0b
7598ee5
a24ef39
870ab56
6c4afca
5cb96d1
baefde1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1382,14 +1382,17 @@ void loadMainFilePreambleMacros(const Preprocessor &PP, | |
| bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, | ||
| const clang::CodeCompleteOptions &Options, | ||
| const SemaCompleteInput &Input, | ||
| IncludeStructure *Includes = nullptr) { | ||
| IncludeStructure *Includes = nullptr, | ||
| std::unique_ptr<CompilerInvocation> CI = nullptr) { | ||
| trace::Span Tracer("Sema completion"); | ||
|
|
||
| IgnoreDiagnostics IgnoreDiags; | ||
| auto CI = buildCompilerInvocation(Input.ParseInput, IgnoreDiags); | ||
| if (!CI) { | ||
| elog("Couldn't create CompilerInvocation"); | ||
| return false; | ||
| CI = buildCompilerInvocation(Input.ParseInput, IgnoreDiags); | ||
| if (!CI) { | ||
| elog("Couldn't create CompilerInvocation"); | ||
| return false; | ||
| } | ||
| } | ||
| auto &FrontendOpts = CI->getFrontendOpts(); | ||
| FrontendOpts.SkipFunctionBodies = true; | ||
|
|
@@ -1615,7 +1618,8 @@ class CodeCompleteFlow { | |
| bool Incomplete = false; // Would more be available with a higher limit? | ||
| CompletionPrefix HeuristicPrefix; | ||
| std::optional<FuzzyMatcher> Filter; // Initialized once Sema runs. | ||
| Range ReplacedRange; | ||
| Range InsertRange; | ||
| std::optional<Range> ReplaceRange; | ||
| std::vector<std::string> QueryScopes; // Initialized once Sema runs. | ||
| std::vector<std::string> AccessibleScopes; // Initialized once Sema runs. | ||
| // Initialized once QueryScopes is initialized, if there are scopes. | ||
|
|
@@ -1750,8 +1754,19 @@ class CodeCompleteFlow { | |
| IsUsingDeclaration = false; | ||
| Filter = FuzzyMatcher(HeuristicPrefix.Name); | ||
| auto Pos = offsetToPosition(Content, Offset); | ||
| ReplacedRange.start = ReplacedRange.end = Pos; | ||
| ReplacedRange.start.character -= HeuristicPrefix.Name.size(); | ||
| InsertRange.start = InsertRange.end = Pos; | ||
| InsertRange.start.character -= HeuristicPrefix.Name.size(); | ||
|
|
||
| if (Opts.EnableInsertReplace) { | ||
| ReplaceRange.emplace(); | ||
| ReplaceRange->start = InsertRange.start; | ||
| // Scan forward past ASCII identifier characters to find replace end. | ||
| size_t ReplaceEnd = Offset; | ||
| while (ReplaceEnd < Content.size() && | ||
| isAsciiIdentifierContinue(Content[ReplaceEnd])) | ||
| ++ReplaceEnd; | ||
| ReplaceRange->end = offsetToPosition(Content, ReplaceEnd); | ||
| } | ||
|
|
||
| llvm::StringMap<SourceParams> ProxSources; | ||
| ProxSources[FileName].Cost = 0; | ||
|
|
@@ -1832,19 +1847,26 @@ class CodeCompleteFlow { | |
| CodeCompleteResult runWithSema() { | ||
| const auto &CodeCompletionRange = CharSourceRange::getCharRange( | ||
| Recorder->CCSema->getPreprocessor().getCodeCompletionTokenRange()); | ||
|
|
||
| const SourceManager &SM = Recorder->CCSema->getSourceManager(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extracting |
||
|
|
||
| // When we are getting completions with an empty identifier, for example | ||
| // std::vector<int> asdf; | ||
| // asdf.^; | ||
| // Then the range will be invalid and we will be doing insertion, use | ||
| // current cursor position in such cases as range. | ||
| if (CodeCompletionRange.isValid()) { | ||
| ReplacedRange = halfOpenToRange(Recorder->CCSema->getSourceManager(), | ||
| CodeCompletionRange); | ||
| InsertRange = halfOpenToRange(SM, CodeCompletionRange); | ||
| } else { | ||
| const auto &Pos = sourceLocToPosition( | ||
| Recorder->CCSema->getSourceManager(), | ||
| Recorder->CCSema->getPreprocessor().getCodeCompletionLoc()); | ||
| ReplacedRange.start = ReplacedRange.end = Pos; | ||
| SM, Recorder->CCSema->getPreprocessor().getCodeCompletionLoc()); | ||
| InsertRange.start = InsertRange.end = Pos; | ||
| } | ||
|
|
||
| if (Opts.EnableInsertReplace) { | ||
| ReplaceRange.emplace(); | ||
| ReplaceRange->start = InsertRange.start; | ||
| ReplaceRange->end = getEndOfCodeCompletionReplace(SM); | ||
| } | ||
| Filter = FuzzyMatcher( | ||
| Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); | ||
|
|
@@ -1874,6 +1896,26 @@ class CodeCompleteFlow { | |
| return toCodeCompleteResult(Top); | ||
| } | ||
|
|
||
| // Returns the LSP position at the end of the identifier suffix after the | ||
| // code completion cursor. | ||
| Position getEndOfCodeCompletionReplace(const SourceManager &SM) { | ||
| const Preprocessor &PP = Recorder->CCSema->getPreprocessor(); | ||
| const LangOptions &LangOpts = Recorder->CCSema->getLangOpts(); | ||
|
|
||
| // Skip past the code completion NUL byte and scan forward through | ||
| // identifier continuation characters (letters, digits, _, $, UCN, | ||
| // unicode). This handles all cases uniformly: with prefix ("vac^1abc"), | ||
| // without prefix ("vec.^asdf"), and digit-starting ("vec.^1abc"). | ||
| const SourceLocation SuffixBegin = | ||
| PP.getCodeCompletionLoc().getLocWithOffset(1); | ||
| Position End = sourceLocToPosition( | ||
| SM, Lexer::findEndOfIdentifierContinuation(SuffixBegin, SM, LangOpts)); | ||
| // Adjust for the NUL byte inserted at the cursor by code completion, | ||
| // which inflates the column by 1. | ||
| End.character--; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels a bit hacky. If anyone has an idea how not to go off by one here, I'm open for suggestions. |
||
| return End; | ||
| } | ||
|
|
||
| CodeCompleteResult | ||
| toCodeCompleteResult(const std::vector<ScoredBundle> &Scored) { | ||
| CodeCompleteResult Output; | ||
|
|
@@ -1885,7 +1927,8 @@ class CodeCompleteFlow { | |
| for (auto &C : Scored) { | ||
| Output.Completions.push_back(toCodeCompletion(C.first)); | ||
| Output.Completions.back().Score = C.second; | ||
| Output.Completions.back().CompletionTokenRange = ReplacedRange; | ||
| Output.Completions.back().CompletionInsertRange = InsertRange; | ||
| Output.Completions.back().CompletionReplaceRange = ReplaceRange; | ||
| if (Opts.Index && !Output.Completions.back().Documentation) { | ||
| for (auto &Cand : C.first) { | ||
| if (Cand.SemaResult && | ||
|
|
@@ -1909,7 +1952,8 @@ class CodeCompleteFlow { | |
| } | ||
| Output.HasMore = Incomplete; | ||
| Output.Context = CCContextKind; | ||
| Output.CompletionRange = ReplacedRange; | ||
| Output.InsertRange = InsertRange; | ||
| Output.ReplaceRange = ReplaceRange; | ||
|
|
||
| // Look up documentation from the index. | ||
| if (Opts.Index) { | ||
|
|
@@ -2230,16 +2274,54 @@ CompletionPrefix guessCompletionPrefix(llvm::StringRef Content, | |
| return Result; | ||
| } | ||
|
|
||
| // If Offset is inside what looks like argument comment (e.g. | ||
| // "/*^*/" or "/* foo = ^*/"), returns the offset pointing past the closing | ||
| // "*/". | ||
| static std::optional<unsigned> | ||
| maybeFunctionArgumentCommentEnd(const PathRef FileName, const unsigned Offset, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe not consistent with finding the beginning of the comment (which is ASCII-only), but it's definitely more correct, and we always have compiler invocation context here, so why not use it. Finding the beginning of the comment can be fixed to use more comprehensive lexing in the future, but it's outside the scope of this change. As searching forward and backward are pretty different actions, it will be unrelated code anyway. |
||
| const llvm::StringRef Content, | ||
| const LangOptions &LangOpts) { | ||
| if (Offset > Content.size()) | ||
| return std::nullopt; | ||
|
|
||
| SourceManagerForFile FileSM(FileName, Content); | ||
| const SourceManager &SM = FileSM.get(); | ||
| const SourceLocation Cursor = SM.getComposedLoc(SM.getMainFileID(), Offset); | ||
| const SourceLocation EndOfSuffix = | ||
| Lexer::findEndOfIdentifierContinuation(Cursor, SM, LangOpts); | ||
| const unsigned EndOfSuffixOffset = SM.getFileOffset(EndOfSuffix); | ||
|
|
||
| const llvm::StringRef Rest = Content.drop_front(EndOfSuffixOffset); | ||
| llvm::StringRef RestTrimmed = Rest.ltrim(); | ||
| // Comment argument pattern: `/* name = */` — skip past optional `=`. | ||
| if (RestTrimmed.starts_with("=")) | ||
| RestTrimmed = RestTrimmed.drop_front(1).ltrim(); | ||
| if (RestTrimmed.starts_with("*/")) | ||
| return EndOfSuffixOffset + (Rest.size() - RestTrimmed.size()) + 2; | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| // Code complete the argument name on "/*" inside function call. | ||
| // Offset should be pointing to the start of the comment, i.e.: | ||
| // OutsideStartOffset should be pointing before the comment, i.e.: | ||
| // foo(^/*, rather than foo(/*^) where the cursor probably is. | ||
| CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset, | ||
| llvm::StringRef Prefix, | ||
| const PreambleData *Preamble, | ||
| const ParseInputs &ParseInput) { | ||
| CodeCompleteResult | ||
| codeCompleteComment(PathRef FileName, const unsigned CursorOffset, | ||
| unsigned OutsideStartOffset, llvm::StringRef Prefix, | ||
| const PreambleData *Preamble, const ParseInputs &ParseInput, | ||
| const CodeCompleteOptions &Opts) { | ||
| if (Preamble == nullptr) // Can't run without Sema. | ||
| return CodeCompleteResult(); | ||
|
|
||
| IgnoreDiagnostics IgnoreDiags; | ||
| auto CI = buildCompilerInvocation(ParseInput, IgnoreDiags); | ||
| if (!CI) | ||
| return CodeCompleteResult(); | ||
|
|
||
| std::optional<unsigned> OutsideEndOffset; | ||
| if (Opts.EnableInsertReplace) | ||
| OutsideEndOffset = maybeFunctionArgumentCommentEnd( | ||
| FileName, CursorOffset, ParseInput.Contents, CI->getLangOpts()); | ||
|
|
||
| clang::CodeCompleteOptions Options; | ||
| Options.IncludeGlobals = false; | ||
| Options.IncludeMacros = false; | ||
|
|
@@ -2250,20 +2332,31 @@ CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset, | |
| // full patch. | ||
| semaCodeComplete( | ||
| std::make_unique<ParamNameCollector>(Options, ParamNames), Options, | ||
| {FileName, Offset, *Preamble, | ||
| {FileName, OutsideStartOffset, *Preamble, | ||
| PreamblePatch::createFullPatch(FileName, ParseInput, *Preamble), | ||
| ParseInput}); | ||
| ParseInput}, | ||
| /*Includes=*/nullptr, std::move(CI)); | ||
| if (ParamNames.empty()) | ||
| return CodeCompleteResult(); | ||
|
|
||
| CodeCompleteResult Result; | ||
| Range CompletionRange; | ||
| Range InsertRange; | ||
| // Skip /* | ||
| Offset += 2; | ||
| CompletionRange.start = offsetToPosition(ParseInput.Contents, Offset); | ||
| CompletionRange.end = | ||
| offsetToPosition(ParseInput.Contents, Offset + Prefix.size()); | ||
| Result.CompletionRange = CompletionRange; | ||
| const unsigned InsideStartOffset = OutsideStartOffset + 2; | ||
| InsertRange.start = offsetToPosition(ParseInput.Contents, InsideStartOffset); | ||
| InsertRange.end = | ||
| offsetToPosition(ParseInput.Contents, InsideStartOffset + Prefix.size()); | ||
| Result.InsertRange = InsertRange; | ||
|
|
||
| if (Opts.EnableInsertReplace) { | ||
| Range ReplaceRange; | ||
| ReplaceRange.start = InsertRange.start; | ||
| ReplaceRange.end = OutsideEndOffset ? offsetToPosition(ParseInput.Contents, | ||
| *OutsideEndOffset) | ||
| : InsertRange.end; | ||
| Result.ReplaceRange = ReplaceRange; | ||
| } | ||
|
|
||
| Result.Context = CodeCompletionContext::CCC_NaturalLanguage; | ||
| for (llvm::StringRef Name : ParamNames) { | ||
| if (!Name.starts_with(Prefix)) | ||
|
|
@@ -2272,7 +2365,8 @@ CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset, | |
| Item.Name = Name.str() + "=*/"; | ||
| Item.FilterText = Item.Name; | ||
| Item.Kind = CompletionItemKind::Text; | ||
| Item.CompletionTokenRange = CompletionRange; | ||
| Item.CompletionInsertRange = InsertRange; | ||
| Item.CompletionReplaceRange = Result.ReplaceRange; | ||
| Item.Origin = SymbolOrigin::AST; | ||
| Result.Completions.push_back(Item); | ||
| } | ||
|
|
@@ -2312,8 +2406,8 @@ CodeCompleteResult codeComplete(PathRef FileName, Position Pos, | |
| // parsing, so we must move back the position before running it, extract | ||
| // information we need and construct completion items ourselves. | ||
| auto CommentPrefix = Content.substr(*OffsetBeforeComment + 2).trim(); | ||
| return codeCompleteComment(FileName, *OffsetBeforeComment, CommentPrefix, | ||
| Preamble, ParseInput); | ||
| return codeCompleteComment(FileName, *Offset, *OffsetBeforeComment, | ||
| CommentPrefix, Preamble, ParseInput, Opts); | ||
| } | ||
|
|
||
| auto Flow = CodeCompleteFlow( | ||
|
|
@@ -2423,7 +2517,9 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const { | |
| } | ||
| LSP.sortText = sortText(Score.Total, FilterText); | ||
| LSP.filterText = FilterText; | ||
| LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name, ""}; | ||
| TextEdit Edit; | ||
| Edit.range = CompletionInsertRange; | ||
| Edit.newText = RequiredQualifier + Name; | ||
| // Merge continuous additionalTextEdits into main edit. The main motivation | ||
| // behind this is to help LSP clients, it seems most of them are confused when | ||
| // they are provided with additionalTextEdits that are consecutive to main | ||
|
|
@@ -2432,19 +2528,34 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const { | |
| // is mainly to help LSP clients again, so that changes do not effect each | ||
| // other. | ||
| for (const auto &FixIt : FixIts) { | ||
| if (FixIt.range.end == LSP.textEdit->range.start) { | ||
| LSP.textEdit->newText = FixIt.newText + LSP.textEdit->newText; | ||
| LSP.textEdit->range.start = FixIt.range.start; | ||
| if (FixIt.range.end == Edit.range.start) { | ||
| Edit.newText = FixIt.newText + Edit.newText; | ||
| Edit.range.start = FixIt.range.start; | ||
| } else { | ||
| LSP.additionalTextEdits.push_back(FixIt); | ||
| } | ||
| } | ||
| if (Opts.EnableSnippets) | ||
| LSP.textEdit->newText += SnippetSuffix; | ||
| Edit.newText += SnippetSuffix; | ||
|
|
||
| // FIXME(kadircet): Do not even fill insertText after making sure textEdit is | ||
| // compatible with most of the editors. | ||
| LSP.insertText = LSP.textEdit->newText; | ||
| LSP.insertText = Edit.newText; | ||
| if (Opts.EnableInsertReplace) { | ||
| assert(CompletionReplaceRange && | ||
| "CompletionReplaceRange must be already set before render() " | ||
| "when EnableInsertReplace is on"); | ||
| InsertReplaceEdit IRE; | ||
| IRE.newText = std::move(Edit.newText); | ||
| IRE.insert = Edit.range; | ||
| IRE.replace = *CompletionReplaceRange; | ||
| // FixIt merging may have extended the insert range start; keep replace | ||
| // range as a superset per LSP spec. | ||
| IRE.replace.start = IRE.insert.start; | ||
| LSP.textEdit = std::move(IRE); | ||
| } else { | ||
| LSP.textEdit = std::move(Edit); | ||
| } | ||
| // Some clients support snippets but work better with plaintext. | ||
| // So if the snippet is trivial, let the client know. | ||
| // https://github.com/clangd/clangd/issues/922 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,6 +202,14 @@ llvm::json::Value toJSON(const TextEdit &P) { | |
| return Result; | ||
| } | ||
|
|
||
| llvm::json::Value toJSON(const InsertReplaceEdit &P) { | ||
| return llvm::json::Object{ | ||
| {"newText", P.newText}, | ||
| {"insert", P.insert}, | ||
| {"replace", P.replace}, | ||
| }; | ||
| } | ||
|
|
||
| bool fromJSON(const llvm::json::Value &Params, ChangeAnnotation &R, | ||
| llvm::json::Path P) { | ||
| llvm::json::ObjectMapper O(Params, P); | ||
|
|
@@ -414,6 +422,8 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R, | |
| break; | ||
| } | ||
| } | ||
| if (auto IRSupport = Item->getBoolean("insertReplaceSupport")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be |
||
| R.InsertReplace = *IRSupport; | ||
| } | ||
| if (auto *ItemKind = Completion->getObject("completionItemKind")) { | ||
| if (auto *ValueSet = ItemKind->get("valueSet")) { | ||
|
|
@@ -1184,7 +1194,8 @@ llvm::json::Value toJSON(const CompletionItem &CI) { | |
| if (CI.insertTextFormat != InsertTextFormat::Missing) | ||
| Result["insertTextFormat"] = static_cast<int>(CI.insertTextFormat); | ||
| if (CI.textEdit) | ||
| Result["textEdit"] = *CI.textEdit; | ||
| Result["textEdit"] = std::visit( | ||
| [](const auto &V) { return llvm::json::Value(V); }, *CI.textEdit); | ||
| if (!CI.additionalTextEdits.empty()) | ||
| Result["additionalTextEdits"] = llvm::json::Array(CI.additionalTextEdits); | ||
| if (CI.deprecated) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the
codeCompleteCommentpath,LangOptsare required before this function call. Thus, adding here the possiblity to callsemaCodeCompletewith an already calculatedCI, to avoid double work.