From fcb6c1e016a130e2314ce1f79e48ba976c17aac9 Mon Sep 17 00:00:00 2001 From: nurullahahmetarikan Date: Tue, 2 Sep 2025 14:10:10 +0300 Subject: [PATCH] [LSPAPI] refactor forgoteetn_this_property_access Issue: https://gitee.com/openharmony/arkcompiler_ets_frontend/issues/ICW1QV Signed-off-by: nurullahahmetarikan --- .../forgotten_this_property_access.h | 5 +- .../forgotten_this_property_access.cpp | 73 ++++---- .../forgotten_this_property_access_test.cpp | 160 ++++++++++++++---- 3 files changed, 168 insertions(+), 70 deletions(-) diff --git a/ets2panda/lsp/include/register_code_fix/forgotten_this_property_access.h b/ets2panda/lsp/include/register_code_fix/forgotten_this_property_access.h index add5dcdfa7..aa51d3595e 100644 --- a/ets2panda/lsp/include/register_code_fix/forgotten_this_property_access.h +++ b/ets2panda/lsp/include/register_code_fix/forgotten_this_property_access.h @@ -31,6 +31,10 @@ public: std::vector GetCodeActions(const CodeFixContext &context) override; CombinedCodeActions GetAllCodeActions(const CodeFixAllContext &codeFixAll) override; + +private: + void DoChanges(ChangeTracker &tracker, es2panda_Context *context, size_t pos); + std::vector GetCodeActionsToFix(const CodeFixContext &context); }; struct Info { @@ -52,7 +56,6 @@ public: }; Info GetInfoThisProp(es2panda_Context *context, size_t offset); -void DoChanges(es2panda_Context *context, ChangeTracker tracker); } // namespace ark::es2panda::lsp diff --git a/ets2panda/lsp/src/register_code_fix/forgotten_this_property_access.cpp b/ets2panda/lsp/src/register_code_fix/forgotten_this_property_access.cpp index abad90b349..090e20ed59 100644 --- a/ets2panda/lsp/src/register_code_fix/forgotten_this_property_access.cpp +++ b/ets2panda/lsp/src/register_code_fix/forgotten_this_property_access.cpp @@ -14,7 +14,6 @@ */ #include "register_code_fix/forgotten_this_property_access.h" -#include #include "code_fix_provider.h" #include "generated/code_fix_register.h" @@ -24,7 +23,7 @@ using codefixes::FORGOTTEN_THIS_PROPERTY_ACCESS; ForgottenThisPropertyAccess::ForgottenThisPropertyAccess() { auto errorCodes = FORGOTTEN_THIS_PROPERTY_ACCESS.GetSupportedCodeNumbers(); - SetErrorCodes({errorCodes.begin(), errorCodes.end()}); // change this to the error code you want to handle + SetErrorCodes({errorCodes.begin(), errorCodes.end()}); SetFixIds({FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId().data()}); } @@ -42,52 +41,48 @@ Info GetInfoThisProp(es2panda_Context *context, size_t offset) return info; } -void DoChanges(es2panda_Context *context, ChangeTracker tracker) +void ForgottenThisPropertyAccess::DoChanges(ChangeTracker &tracker, es2panda_Context *context, size_t pos) { - auto ctx = reinterpret_cast(context); - const auto impl = es2panda_GetImpl(ES2PANDA_LIB_VERSION); - - const auto &diagnostics = - ctx->diagnosticEngine->GetDiagnosticStorage(ark::es2panda::util::DiagnosticType::SEMANTIC); + auto info = GetInfoThisProp(context, pos); + auto node = info.GetNode(); + if (node == nullptr) { + return; + } - for (const auto &diagnostic : diagnostics) { - auto index = ark::es2panda::lexer::LineIndex(ctx->parserProgram->SourceCode()); - auto offset = index.GetOffset( - ark::es2panda::lexer::SourceLocation(diagnostic->Line(), diagnostic->Offset(), ctx->parserProgram)); - auto node = ark::es2panda::lsp::GetTouchingToken(context, offset, false); - es2panda_AstNode *thisExpr = impl->CreateThisExpression(context); - es2panda_AstNode *memberExpr = - impl->CreateMemberExpression(context, thisExpr, reinterpret_cast(node), - MEMBER_EXPRESSION_KIND_PROPERTY_ACCESS, false, false); - impl->AstNodeSetParent(context, thisExpr, memberExpr); - impl->AstNodeSetParent(context, reinterpret_cast(node), memberExpr); - auto memNode = reinterpret_cast(memberExpr); - if (memNode == nullptr) { - continue; - } + const auto impl = es2panda_GetImpl(ES2PANDA_LIB_VERSION); + es2panda_AstNode *thisExpr = impl->CreateThisExpression(context); + es2panda_AstNode *memberExpr = + impl->CreateMemberExpression(context, thisExpr, reinterpret_cast(node), + MEMBER_EXPRESSION_KIND_PROPERTY_ACCESS, false, false); + impl->AstNodeSetParent(context, thisExpr, memberExpr); + impl->AstNodeSetParent(context, reinterpret_cast(node), memberExpr); + auto memNode = reinterpret_cast(memberExpr); + if (memNode != nullptr) { tracker.ReplaceNode(context, node, memNode, {}); } } +std::vector ForgottenThisPropertyAccess::GetCodeActionsToFix(const CodeFixContext &context) +{ + TextChangesContext textChangesContext = {context.host, context.formatContext, context.preferences}; + auto fileTextChanges = ChangeTracker::With( + textChangesContext, [&](ChangeTracker &tracker) { DoChanges(tracker, context.context, context.span.start); }); + return fileTextChanges; +} + std::vector ForgottenThisPropertyAccess::GetCodeActions(const CodeFixContext &context) { std::vector returnedActions; - - const auto info = GetInfoThisProp(context.context, context.span.start); - if (info.GetNode() == nullptr) { - return {}; + auto changes = GetCodeActionsToFix(context); + if (!changes.empty()) { + CodeFixAction action; + action.fixName = FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId().data(); + action.description = "Add 'this.' to property access"; + action.fixId = FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId().data(); + action.fixAllDescription = "Add 'this.' to all property accesses in the file"; + action.changes.insert(action.changes.end(), changes.begin(), changes.end()); + returnedActions.push_back(action); } - TextChangesContext textChangesContext {context.host, context.formatContext, context.preferences}; - const auto changes = - ChangeTracker::With(textChangesContext, [&](ChangeTracker &tracker) { DoChanges(context.context, tracker); }); - std::vector actions; - CodeFixAction action; - action.fixName = FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId().data(); - action.description = "Add 'this.' to property access"; - action.fixId = FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId().data(); - action.changes.insert(action.changes.end(), changes.begin(), changes.end()); - action.fixAllDescription = "Add 'this.' to all property accesses in the file"; - returnedActions.push_back(action); return returnedActions; } @@ -98,7 +93,7 @@ CombinedCodeActions ForgottenThisPropertyAccess::GetAllCodeActions(const CodeFix [&](ChangeTracker &tracker, const DiagnosticWithLocation &diag) { auto info = GetInfoThisProp(codeFixAll.context, diag.GetStart()); if (info.GetNode() != nullptr) { - DoChanges(codeFixAll.context, tracker); + DoChanges(tracker, codeFixAll.context, diag.GetStart()); } }); diff --git a/ets2panda/test/unit/lsp/forgotten_this_property_access_test.cpp b/ets2panda/test/unit/lsp/forgotten_this_property_access_test.cpp index 058686cbb1..114cf5169d 100644 --- a/ets2panda/test/unit/lsp/forgotten_this_property_access_test.cpp +++ b/ets2panda/test/unit/lsp/forgotten_this_property_access_test.cpp @@ -12,53 +12,153 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "gtest/gtest.h" -#include "lsp_api_test.h" -#include + +#include #include #include "lsp/include/register_code_fix/forgotten_this_property_access.h" +#include "generated/code_fix_register.h" +#include "lsp/include/internal_api.h" +#include "lsp_api_test.h" +#include "public/es2panda_lib.h" namespace { -class ForgottenThisPropertyAccessTests : public LSPAPITests {}; -TEST_F(ForgottenThisPropertyAccessTests, ForgottenThisPropertyAccessTests_GetInfo) +using ark::es2panda::lsp::Initializer; +using ark::es2panda::lsp::codefixes::FORGOTTEN_THIS_PROPERTY_ACCESS; + +constexpr std::string_view EXPECTED_FIX_NAME = FORGOTTEN_THIS_PROPERTY_ACCESS.GetFixId(); +constexpr auto ERROR_CODES = FORGOTTEN_THIS_PROPERTY_ACCESS.GetSupportedCodeNumbers(); +constexpr std::string_view EXPECTED_FIX_DESCRIPTION = "Add 'this.' to property access"; +constexpr int DEFAULT_THROTTLE = 20; +constexpr int FORGOTTEN_THIS_PROPERTY_ACCESS_ID = 145; +class ForgottenThisPropertyAccessTests : 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 + { + return false; + } + }; + + static NullCancellationToken &GetNullHost() + { + static NullCancellationToken instance; + return instance; + } +}; + +TEST_F(ForgottenThisPropertyAccessTests, TestForgottenThisPropertyAccess_BasicCase) { - const char *source = R"(class Person { + Initializer initializer = Initializer(); + es2panda_Context *ctx = initializer.CreateContext("ForgottenThisPropertyAccess_Basic.ets", ES2PANDA_STATE_CHECKED, + R"(class Person { name = "Alice"; greet() { console.log(name); } -} - )"; - ark::es2panda::lsp::Initializer initializer = ark::es2panda::lsp::Initializer(); - es2panda_Context *context = - initializer.CreateContext("ForgottenThisPropertyAccessTests_GetInfo.ets", ES2PANDA_STATE_CHECKED, source); - - auto ctx = reinterpret_cast(context); - const auto impl = es2panda_GetImpl(ES2PANDA_LIB_VERSION); +})"); + auto ctxInternal = reinterpret_cast(ctx); const auto &diagnostics = - ctx->diagnosticEngine->GetDiagnosticStorage(ark::es2panda::util::DiagnosticType::SEMANTIC); + ctxInternal->diagnosticEngine->GetDiagnosticStorage(ark::es2panda::util::DiagnosticType::SEMANTIC); + bool foundPropAccessError = false; for (const auto &diagnostic : diagnostics) { - auto index = ark::es2panda::lexer::LineIndex(ctx->parserProgram->SourceCode()); - auto offset = index.GetOffset( - ark::es2panda::lexer::SourceLocation(diagnostic->Line(), diagnostic->Offset(), ctx->parserProgram)); - auto node = ark::es2panda::lsp::GetTouchingToken(context, offset, false); - es2panda_AstNode *thisExpr = impl->CreateThisExpression(context); - es2panda_AstNode *memberExpr = - impl->CreateMemberExpression(context, thisExpr, reinterpret_cast(node), - MEMBER_EXPRESSION_KIND_PROPERTY_ACCESS, false, false); - impl->AstNodeSetParent(context, thisExpr, memberExpr); - impl->AstNodeSetParent(context, reinterpret_cast(node), memberExpr); - auto *memNode = reinterpret_cast(memberExpr); - if (memNode == nullptr) { - continue; + if (diagnostic->GetId() == FORGOTTEN_THIS_PROPERTY_ACCESS_ID) { + foundPropAccessError = true; + break; } - ASSERT_EQ(memNode->DumpEtsSrc(), "this.name"); } - initializer.DestroyContext(context); + ASSERT_TRUE(foundPropAccessError); + + std::string expectedFileName = "ForgottenThisPropertyAccess_Basic.ets"; + const size_t start = LineColToPos(ctx, 4, 13); + const size_t length = 4; + std::vector errorCodes(ERROR_CODES.begin(), ERROR_CODES.end()); + CodeFixOptions options = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; + + auto result = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(ctx, start, length, errorCodes, options); + if (result.empty()) { + initializer.DestroyContext(ctx); + return; + } + + if (!result.empty() && !result[0].changes_.empty()) { + if (!result[0].changes_[0].textChanges.empty()) { + ASSERT_TRUE(result[0].changes_[0].textChanges[0].newText.find("this.name") != std::string::npos); + } + ASSERT_EQ(result[0].fixName_, EXPECTED_FIX_NAME); + ASSERT_EQ(result[0].fixId_, EXPECTED_FIX_NAME); + ASSERT_EQ(result[0].description_, EXPECTED_FIX_DESCRIPTION); + } + + initializer.DestroyContext(ctx); +} + +TEST_F(ForgottenThisPropertyAccessTests, TestForgottenThisPropertyAccess_MultipleProperties) +{ + Initializer initializer = Initializer(); + es2panda_Context *ctx = + initializer.CreateContext("ForgottenThisPropertyAccess_Multiple.ets", ES2PANDA_STATE_CHECKED, + R"(class Person { +name = "Alice"; +age = 25; +greet() { +console.log(name + " is " + age); +} +})"); + + auto ctxInternal = reinterpret_cast(ctx); + const auto &diagnostics = + ctxInternal->diagnosticEngine->GetDiagnosticStorage(ark::es2panda::util::DiagnosticType::SEMANTIC); + + constexpr size_t expectedDiagnosticCount = 2; + ASSERT_GE(diagnostics.size(), expectedDiagnosticCount); + + std::string expectedFileName = "ForgottenThisPropertyAccess_Multiple.ets"; + const size_t start1 = LineColToPos(ctx, 5, 13); + const size_t start2 = LineColToPos(ctx, 5, 29); + const size_t length = 4; + + std::vector errorCodes(ERROR_CODES.begin(), ERROR_CODES.end()); + CodeFixOptions options = {CreateNonCancellationToken(), ark::es2panda::lsp::FormatCodeSettings(), {}}; + + auto result1 = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(ctx, start1, length, errorCodes, options); + ASSERT_FALSE(result1.empty()) << "Fix should be found for 'name' at position (5,13)"; + ASSERT_FALSE(result1[0].changes_.empty()) << "Changes should exist for 'name' fix"; + ASSERT_FALSE(result1[0].changes_[0].textChanges.empty()) << "Text changes should exist for 'name' fix"; + + ASSERT_TRUE(result1[0].changes_[0].textChanges[0].newText.find("this.name") != std::string::npos); + ASSERT_EQ(result1[0].fixName_, EXPECTED_FIX_NAME); + ASSERT_EQ(result1[0].fixId_, EXPECTED_FIX_NAME); + ASSERT_EQ(result1[0].description_, EXPECTED_FIX_DESCRIPTION); + + auto result2 = ark::es2panda::lsp::GetCodeFixesAtPositionImpl(ctx, start2, length, errorCodes, options); + ASSERT_FALSE(result2.empty()) << "Fix should be found for 'age' at position (5,29)"; + ASSERT_FALSE(result2[0].changes_.empty()) << "Changes should exist for 'age' fix"; + ASSERT_FALSE(result2[0].changes_[0].textChanges.empty()) << "Text changes should exist for 'age' fix"; + + ASSERT_TRUE(result2[0].changes_[0].textChanges[0].newText.find("this.age") != std::string::npos); + ASSERT_EQ(result2[0].fixName_, EXPECTED_FIX_NAME); + ASSERT_EQ(result2[0].fixId_, EXPECTED_FIX_NAME); + ASSERT_EQ(result2[0].description_, EXPECTED_FIX_DESCRIPTION); + + initializer.DestroyContext(ctx); } } // namespace \ No newline at end of file -- Gitee