From bb34cd73d778907c24dedd6dde6dd0037d3d8903 Mon Sep 17 00:00:00 2001 From: dogasahin_7f63 Date: Thu, 4 Sep 2025 08:43:45 +0300 Subject: [PATCH] [LSP] refactor AddMissingNewOperator Issue: https://gitee.com/openharmony/arkcompiler_ets_frontend/issues/ICUXNM Signed-off-by: dogasahin_7f63 --- ets2panda/lsp/include/code_fix_provider.h | 4 +- ets2panda/lsp/src/code_fix_provider.cpp | 25 +-- .../add_missing_new_operator.cpp | 44 ++++- .../test/unit/lsp/add_local_variable_test.cpp | 18 +- .../lsp/add_missing_new_operator_test.cpp | 163 ++++++++++++------ 5 files changed, 170 insertions(+), 84 deletions(-) diff --git a/ets2panda/lsp/include/code_fix_provider.h b/ets2panda/lsp/include/code_fix_provider.h index 567be5aa86..5bd4ef3dd8 100644 --- a/ets2panda/lsp/include/code_fix_provider.h +++ b/ets2panda/lsp/include/code_fix_provider.h @@ -28,11 +28,11 @@ namespace ark::es2panda::lsp { using ark::es2panda::lsp::codefixes::DiagnosticCode; class CodeFixProvider { private: - std::unordered_map> errorCodeToFixes_; + std::unordered_map>> errorCodeToFixes_; std::unordered_map> fixIdToRegistration_; public: - std::unordered_map> GetErrorCodeToFixes() const + std::unordered_map>> GetErrorCodeToFixes() const { return errorCodeToFixes_; } diff --git a/ets2panda/lsp/src/code_fix_provider.cpp b/ets2panda/lsp/src/code_fix_provider.cpp index ba0150a575..ab5aebc407 100644 --- a/ets2panda/lsp/src/code_fix_provider.cpp +++ b/ets2panda/lsp/src/code_fix_provider.cpp @@ -25,19 +25,21 @@ namespace ark::es2panda::lsp { // Registers a new code fix and maps it to its diagnostic error codes and fix IDs -void CodeFixProvider::RegisterCodeFix(const std::string &aliasName, std::unique_ptr registration) +void CodeFixProvider::RegisterCodeFix([[maybe_unused]] const std::string &aliasName, + std::unique_ptr registration) { - (void)aliasName; ASSERT(!aliasName.empty()); auto shared = std::shared_ptr(std::move(registration)); + // Allow multiple fixes for the same error code for (auto error : shared->GetErrorCodes()) { - errorCodeToFixes_.emplace(std::to_string(error), shared); + auto &vec = errorCodeToFixes_[std::to_string(error)]; + vec.push_back(shared); } - if (!shared->GetFixIds().empty()) { - for (const auto &fixId : shared->GetFixIds()) { - fixIdToRegistration_.emplace(fixId, shared); - } + + // Fix IDs remain one-to-one, so we keep the existing map + for (const auto &fixId : shared->GetFixIds()) { + fixIdToRegistration_.emplace(fixId, shared); } } @@ -219,10 +221,11 @@ std::vector CodeFixProvider::GetFixes(const CodeFixContext &conte std::vector result; auto it = errorCodeToFixes_.find(std::to_string(context.errorCode)); if (it != errorCodeToFixes_.end()) { - const auto ®istrations = it->second; - if (registrations) { - auto actions = registrations->GetCodeActions(context); - result.insert(result.end(), actions.begin(), actions.end()); + for (auto ® : it->second) { + if (reg) { + auto actions = reg->GetCodeActions(context); + result.insert(result.end(), actions.begin(), actions.end()); + } } } return result; diff --git a/ets2panda/lsp/src/register_code_fix/add_missing_new_operator.cpp b/ets2panda/lsp/src/register_code_fix/add_missing_new_operator.cpp index 8c678ce14e..9c1752ade4 100644 --- a/ets2panda/lsp/src/register_code_fix/add_missing_new_operator.cpp +++ b/ets2panda/lsp/src/register_code_fix/add_missing_new_operator.cpp @@ -14,21 +14,47 @@ */ #include "lsp/include/internal_api.h" +#include "generated/code_fix_register.h" #include "lsp/include/code_fix_provider.h" #include "lsp/include/register_code_fix/add_missing_new_operator.h" namespace ark::es2panda::lsp { -const int G_ADD_MISSING_NEW_OPERATOR_CODE = 1022; +using codefixes::FIX_ADD_MISSING_NEW_OPERATOR; FixAddMissingNewOperator::FixAddMissingNewOperator() { - const char *fixId = "FixAddMissingNewOperator"; - SetErrorCodes({G_ADD_MISSING_NEW_OPERATOR_CODE}); - SetFixIds({fixId}); + auto errorCodes = FIX_ADD_MISSING_NEW_OPERATOR.GetSupportedCodeNumbers(); + SetErrorCodes({errorCodes.begin(), errorCodes.end()}); + SetFixIds({FIX_ADD_MISSING_NEW_OPERATOR.GetFixId().data()}); } bool FixAddMissingNewOperator::IsValidTarget(const ir::AstNode *node) { - return node != nullptr && node->IsCallExpression(); + if (node == nullptr || !node->IsCallExpression()) { + return false; + } + + const auto *call = node->AsCallExpression(); + const auto *callee = call->Callee(); + if (callee == nullptr) { + return false; + } + + if (!callee->IsIdentifier()) { + return false; + } + const ir::Identifier *id = callee->AsIdentifier(); + + auto *var = id->Variable(); + if (var == nullptr || var->Declaration() == nullptr) { + return false; + } + + const auto *decl = var->Declaration()->Node(); + if (decl == nullptr || (!decl->IsClassDeclaration() && !decl->IsClassDefinition())) { + return false; + } + + return true; } void FixAddMissingNewOperator::MakeChange(ChangeTracker &changeTracker, es2panda_Context *context, size_t pos, @@ -43,7 +69,7 @@ void FixAddMissingNewOperator::MakeChange(ChangeTracker &changeTracker, es2panda while (callExpr != nullptr && !callExpr->IsCallExpression()) { callExpr = callExpr->Parent(); } - if (!IsValidTarget(callExpr)) { + if (callExpr == nullptr || !IsValidTarget(callExpr)) { return; } auto *call = const_cast(callExpr->AsCallExpression()); @@ -86,10 +112,10 @@ std::vector FixAddMissingNewOperator::GetCodeActions(const CodeFi auto changes = GetCodeActionsToFix(context); if (!changes.empty()) { CodeFixAction codeAction; - codeAction.fixName = "addMissingNewOperator"; + codeAction.fixName = FIX_ADD_MISSING_NEW_OPERATOR.GetFixId().data(); codeAction.description = "Add missing 'new' operator to constructor call"; codeAction.changes = changes; - codeAction.fixId = "AddMissingNewOperator"; + codeAction.fixId = FIX_ADD_MISSING_NEW_OPERATOR.GetFixId().data(); returnedActions.push_back(codeAction); } return returnedActions; @@ -101,5 +127,5 @@ CombinedCodeActions FixAddMissingNewOperator::GetAllCodeActions([[maybe_unused]] } // NOLINTNEXTLINE(fuchsia-statically-constructed-objects, cert-err58-cpp) -AutoCodeFixRegister g_addMissingNew("AddMissingNewOperator"); +AutoCodeFixRegister g_addMissingNew(FIX_ADD_MISSING_NEW_OPERATOR.GetFixId().data()); } // namespace ark::es2panda::lsp \ No newline at end of file diff --git a/ets2panda/test/unit/lsp/add_local_variable_test.cpp b/ets2panda/test/unit/lsp/add_local_variable_test.cpp index d0dda3ba0d..0a3b905e43 100644 --- a/ets2panda/test/unit/lsp/add_local_variable_test.cpp +++ b/ets2panda/test/unit/lsp/add_local_variable_test.cpp @@ -104,13 +104,13 @@ function calculate() { const size_t expectedTextChangeStart = 23; const size_t expectedTextChangeLength = 0; const std::string expectedNewText = " let result: Double;"; - const int expectedFixResultSize = 1; + const int expectedFixResultSize = 3; std::vector errorCodes(FUNCTION_ERROR_CODES.begin(), FUNCTION_ERROR_CODES.end()); CodeFixOptions emptyOptions = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; auto fixResult = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + length, errorCodes, emptyOptions); - ASSERT_EQ(fixResult.size(), expectedFixResultSize); + EXPECT_EQ(fixResult.size(), expectedFixResultSize); ExpectedCodeFixInfo expected = { expectedTextChangeStart, expectedTextChangeLength, filePaths[0], @@ -140,13 +140,13 @@ class MyComponent { const size_t expectedTextChangeStart = 20; const size_t expectedTextChangeLength = 0; const std::string expectedNewText = " title: String;"; - const int expectedFixResultSize = 1; + const int expectedFixResultSize = 2; std::vector errorCodes(CLASS_ERROR_CODES.begin(), CLASS_ERROR_CODES.end()); CodeFixOptions emptyOptions = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; auto fixResult = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + length, errorCodes, emptyOptions); - ASSERT_EQ(fixResult.size(), expectedFixResultSize); + EXPECT_EQ(fixResult.size(), expectedFixResultSize); ExpectedCodeFixInfo expected = {expectedTextChangeStart, expectedTextChangeLength, filePaths[0], expectedNewText, EXPECTED_CLASS_FIX_NAME, EXPECTED_CLASS_FIX_DESCRIPTION}; @@ -181,13 +181,13 @@ class MyComponent { const size_t expectedTextChangeStart = 20; const size_t expectedTextChangeLength = 0; const std::string expectedNewText = " title: String;"; - const int expectedFixResultSize = 1; + const int expectedFixResultSize = 2; std::vector errorCodes(CLASS_ERROR_CODES.begin(), CLASS_ERROR_CODES.end()); CodeFixOptions emptyOptions = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; auto fixResult = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + length, errorCodes, emptyOptions); - ASSERT_EQ(fixResult.size(), expectedFixResultSize); + EXPECT_EQ(fixResult.size(), expectedFixResultSize); ExpectedCodeFixInfo expected = {expectedTextChangeStart, expectedTextChangeLength, filePaths[0], expectedNewText, EXPECTED_CLASS_FIX_NAME, EXPECTED_CLASS_FIX_DESCRIPTION}; @@ -222,7 +222,7 @@ function process() { const size_t expectedTextChangeStart2 = 21; const size_t expectedTextChangeLength2 = 0; const std::string expectedNewText2 = " let message: String;"; - const int expectedFixResultSize = 1; + const int expectedFixResultSize = 3; std::vector errorCodes(FUNCTION_ERROR_CODES.begin(), FUNCTION_ERROR_CODES.end()); CodeFixOptions emptyOptions = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; @@ -232,8 +232,8 @@ function process() { auto fixResult2 = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start2, start2 + length2, errorCodes, emptyOptions); - ASSERT_EQ(fixResult1.size(), expectedFixResultSize); - ASSERT_EQ(fixResult2.size(), expectedFixResultSize); + EXPECT_EQ(fixResult1.size(), expectedFixResultSize); + EXPECT_EQ(fixResult2.size(), expectedFixResultSize); ExpectedCodeFixInfo expected1 = { expectedTextChangeStart1, expectedTextChangeLength1, filePaths[0], diff --git a/ets2panda/test/unit/lsp/add_missing_new_operator_test.cpp b/ets2panda/test/unit/lsp/add_missing_new_operator_test.cpp index c66a9efc64..86750325e8 100644 --- a/ets2panda/test/unit/lsp/add_missing_new_operator_test.cpp +++ b/ets2panda/test/unit/lsp/add_missing_new_operator_test.cpp @@ -13,20 +13,57 @@ * limitations under the License. */ -#include "gtest/gtest.h" #include "lsp_api_test.h" -#include "public/es2panda_lib.h" -#include "lsp/include/internal_api.h" -#include "lsp/include/code_fixes/code_fix_types.h" + +#include + +#include "lsp/include/api.h" +#include "lsp/include/cancellation_token.h" #include "lsp/include/register_code_fix/add_missing_new_operator.h" namespace { + +using ark::es2panda::lsp::Initializer; +using ark::es2panda::lsp::codefixes::FIX_ADD_MISSING_NEW_OPERATOR; + +constexpr std::string_view EXPECTED_FIX_NAME = FIX_ADD_MISSING_NEW_OPERATOR.GetFixId(); +constexpr auto ERROR_CODES = FIX_ADD_MISSING_NEW_OPERATOR.GetSupportedCodeNumbers(); +constexpr std::string_view EXPECTED_FIX_DESCRIPTION = "Add missing 'new' operator to constructor call"; + +// ---- Test 1: Adds 'new' to constructor call ---- +constexpr size_t T1_ERROR_LINE = 3; // "let a:Foo = Foo();" +constexpr size_t T1_ERROR_COLUMN = 13; // column of 'F' in the call "Foo()" +constexpr size_t T1_ERROR_LENGTH = 0; + +constexpr std::string_view T1_EXPECTED_REPLACEMENT = "new Foo()"; + +// ---- Test 2: Already has 'new' -> no action ---- +constexpr size_t T2_ERROR_LINE = 5; // "const obj = new Foo();" +constexpr size_t T2_ERROR_COLUMN = 17; // column of 'F' in "Foo()" (after 'new ') +constexpr size_t T2_ERROR_LENGTH = 0; + +// ---- Test 3: Non-class function call -> no action ---- +constexpr size_t T3_ERROR_LINE = 3; // "foo();" +constexpr size_t T3_ERROR_COLUMN = 1; // start of "foo()" +constexpr size_t T3_ERROR_LENGTH = 0; + constexpr int DEFAULT_THROTTLE = 20; -const size_t MISSING_NEW_IDX = 26; -const size_t ALREADY_HAS_NEW_IDX = 48; -const size_t SKIP_NON_CLASS_FUNC_CALLS_IDX = 48; + class FixAddMissingNewOperatorTests : public LSPAPITests { public: + static ark::es2panda::lsp::CancellationToken CreateNonCancellationToken() + { + return ark::es2panda::lsp::CancellationToken(DEFAULT_THROTTLE, &GetNullHost()); + } + + static size_t LineColToPos(es2panda_Context *context, const size_t line, const size_t col) + { + auto ctx = reinterpret_cast(context); + auto index = ark::es2panda::lexer::LineIndex(ctx->parserProgram->SourceCode()); + return index.GetOffset(ark::es2panda::lexer::SourceLocation(line, col, ctx->parserProgram)); + } + +private: class NullCancellationToken : public ark::es2panda::lsp::HostCancellationToken { public: bool IsCancellationRequested() override @@ -34,72 +71,92 @@ public: return false; } }; - static ark::es2panda::lsp::CancellationToken CreateToken() - { - static NullCancellationToken nullToken; - return ark::es2panda::lsp::CancellationToken(DEFAULT_THROTTLE, &nullToken); - } - static ark::es2panda::lsp::CodeFixContext CreateCodeFixContext(es2panda_Context *ctx, size_t pos) + + static NullCancellationToken &GetNullHost() { - ark::es2panda::lsp::RulesMap rules; - ark::es2panda::lsp::FormatCodeSettings formatSettings; - ark::es2panda::lsp::UserPreferences preferences; - LanguageServiceHost host; - ark::es2panda::lsp::FormatContext fmtCtx {formatSettings, rules}; - TextChangesContext textCtx {host, fmtCtx, preferences}; - return ark::es2panda::lsp::CodeFixContext {{textCtx, ctx, CreateToken()}, 0, TextSpan {pos, 0}}; + static NullCancellationToken instance; + return instance; } }; TEST_F(FixAddMissingNewOperatorTests, AddsNewKeywordToConstructorCall) { - ark::es2panda::lsp::Initializer initializer; - const std::string sourceCode = R"( + std::vector fileNames = {"AddNew_ToConstructorCall.ets"}; + std::vector fileContents = {R"( class Foo {} let a:Foo = Foo(); -)"; - es2panda_Context *ctx = initializer.CreateContext("missing_new_operator_constructor_call.ets", - ES2PANDA_STATE_CHECKED, sourceCode.c_str()); - ark::es2panda::lsp::FixAddMissingNewOperator fix; - ark::es2panda::lsp::CodeFixContext context = CreateCodeFixContext(ctx, MISSING_NEW_IDX); - auto actions = fix.GetCodeActions(context); - ASSERT_EQ(actions.size(), 1); - const auto &change = actions[0].changes[0].textChanges[0]; - EXPECT_TRUE(change.newText.find("new ") != std::string::npos); - initializer.DestroyContext(ctx); +)"}; + + auto filePaths = CreateTempFile(fileNames, fileContents); + ASSERT_EQ(fileNames.size(), filePaths.size()); + Initializer initializer; + auto *context = initializer.CreateContext(filePaths[0].c_str(), ES2PANDA_STATE_CHECKED); + size_t start = LineColToPos(context, T1_ERROR_LINE, T1_ERROR_COLUMN); + std::vector errorCodes(ERROR_CODES.begin(), ERROR_CODES.end()); + CodeFixOptions options {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; + auto fixes = + ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + T1_ERROR_LENGTH, errorCodes, options); + const size_t c1 = 1; + ASSERT_EQ(fixes.size(), c1); + ASSERT_EQ(fixes[0].fixName_, EXPECTED_FIX_NAME); + ASSERT_EQ(fixes[0].description_, EXPECTED_FIX_DESCRIPTION); + ASSERT_EQ(fixes[0].changes_[0].fileName, filePaths[0]); + const auto &textChanges = fixes[0].changes_[0].textChanges; + ASSERT_EQ(textChanges.size(), c1); + const auto &tc = textChanges[0]; + EXPECT_EQ(tc.newText, T1_EXPECTED_REPLACEMENT); + initializer.DestroyContext(context); } TEST_F(FixAddMissingNewOperatorTests, SkipsValidNewCall) { - ark::es2panda::lsp::Initializer initializer; - const std::string sourceCode = R"( + std::vector fileNames = {"Skip_WhenAlreadyHasNew.ets"}; + std::vector fileContents = {R"( class Foo { -constructor() {} + constructor() {} } const obj = new Foo(); - )"; - es2panda_Context *ctx = - initializer.CreateContext("missing_new_operator_skip_valid.ets", ES2PANDA_STATE_CHECKED, sourceCode.c_str()); - ark::es2panda::lsp::FixAddMissingNewOperator fix; - ark::es2panda::lsp::CodeFixContext context = CreateCodeFixContext(ctx, ALREADY_HAS_NEW_IDX); - auto actions = fix.GetCodeActions(context); - EXPECT_TRUE(actions.empty()); - initializer.DestroyContext(ctx); +)"}; + + auto filePaths = CreateTempFile(fileNames, fileContents); + ASSERT_EQ(fileNames.size(), filePaths.size()); + + Initializer initializer; + auto *context = initializer.CreateContext(filePaths[0].c_str(), ES2PANDA_STATE_CHECKED); + + size_t start = LineColToPos(context, T2_ERROR_LINE, T2_ERROR_COLUMN); + std::vector errorCodes(ERROR_CODES.begin(), ERROR_CODES.end()); + CodeFixOptions options {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; + + auto fixes = + ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + T2_ERROR_LENGTH, errorCodes, options); + EXPECT_TRUE(fixes.empty()); + + initializer.DestroyContext(context); } TEST_F(FixAddMissingNewOperatorTests, SkipsNonClassFunctionCalls) { - ark::es2panda::lsp::Initializer initializer; - const std::string sourceCode = R"( + std::vector fileNames = {"Skip_NonClassFunctionCalls.ets"}; + std::vector fileContents = {R"( function foo() {} foo(); - )"; - es2panda_Context *ctx = initializer.CreateContext("missing_new_operator_skip_non_class_func_calls.ets", - ES2PANDA_STATE_CHECKED, sourceCode.c_str()); - ark::es2panda::lsp::FixAddMissingNewOperator fix; - ark::es2panda::lsp::CodeFixContext context = CreateCodeFixContext(ctx, SKIP_NON_CLASS_FUNC_CALLS_IDX); - auto actions = fix.GetCodeActions(context); - EXPECT_TRUE(actions.empty()); - initializer.DestroyContext(ctx); +)"}; + + auto filePaths = CreateTempFile(fileNames, fileContents); + ASSERT_EQ(fileNames.size(), filePaths.size()); + + Initializer initializer; + auto *context = initializer.CreateContext(filePaths[0].c_str(), ES2PANDA_STATE_CHECKED); + + size_t start = LineColToPos(context, T3_ERROR_LINE, T3_ERROR_COLUMN); + std::vector errorCodes(ERROR_CODES.begin(), ERROR_CODES.end()); + CodeFixOptions options {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; + + auto fixes = + ark::es2panda::lsp::GetCodeFixesAtPositionImpl(context, start, start + T3_ERROR_LENGTH, errorCodes, options); + EXPECT_TRUE(fixes.empty()); + + initializer.DestroyContext(context); } } // namespace \ No newline at end of file -- Gitee