Skip to content

Commit

Permalink
fix: Implicit workspaces should not attempt reading configuration files
Browse files Browse the repository at this point in the history
  • Loading branch information
slavek-kucera authored Sep 1, 2023
1 parent 75e077b commit 239bf21
Show file tree
Hide file tree
Showing 16 changed files with 268 additions and 240 deletions.
1 change: 1 addition & 0 deletions clients/vscode-hlasmplugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- "Diagnostics suppressed" informational message may be incorrectly generated
- Auto-select WebAssembly image on platforms without native support
- Querying current directory fails on Windows (WASM)
- Implicit workspaces should not attempt reading configuration files

## [1.9.0](https://github.com/eclipse-che4z/che-che4z-lsp-for-hlasm/compare/1.8.0...1.9.0) (2023-08-03)

Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/workspace_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ class workspace_manager_impl final : public workspace_manager,
struct opened_workspace
{
opened_workspace(const resource_location& location,
const std::string& name,
const std::string&,
workspaces::file_manager& file_manager,
const lib_config& global_config,
external_configuration_requests* ecr)
: ws(location, name, file_manager, global_config, settings, ecr)
: ws(location, file_manager, global_config, settings, ecr)
{}
opened_workspace(workspaces::file_manager& file_manager, const lib_config& global_config)
: ws(file_manager, global_config, settings)
Expand Down
6 changes: 2 additions & 4 deletions parser_library/src/workspaces/workspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,11 @@ struct workspace_parse_lib_provider final : public parse_lib_provider
};

workspace::workspace(const resource_location& location,
const std::string& name,
file_manager& file_manager,
const lib_config& global_config,
const shared_json& global_settings,
external_configuration_requests* ecr)
: name_(name)
, location_(location.lexically_normal())
: location_(location.lexically_normal())
, file_manager_(file_manager)
, fm_vfm_(file_manager_, location)
, implicit_proc_grp("pg_implicit", {}, {})
Expand All @@ -284,7 +282,7 @@ workspace::workspace(file_manager& file_manager,
const shared_json& global_settings,
std::shared_ptr<library> implicit_library,
external_configuration_requests* ecr)
: workspace(resource_location(""), "", file_manager, global_config, global_settings, ecr)
: workspace(resource_location(), file_manager, global_config, global_settings, ecr)
{
opened_ = true;
if (implicit_library)
Expand Down
2 changes: 0 additions & 2 deletions parser_library/src/workspaces/workspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class workspace : public diagnosable_impl
std::shared_ptr<library> implicit_library = nullptr,
external_configuration_requests* ecr = nullptr);
workspace(const resource_location& location,
const std::string& name,
file_manager& file_manager,
const lib_config& global_config,
const shared_json& global_settings,
Expand Down Expand Up @@ -142,7 +141,6 @@ class workspace : public diagnosable_impl
void invalidate_external_configuration(const resource_location& url);

private:
std::string name_;
resource_location location_;
file_manager& file_manager_;
file_manager_vfm fm_vfm_;
Expand Down
19 changes: 11 additions & 8 deletions parser_library/src/workspaces/workspace_configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ workspace_configuration::~workspace_configuration() = default;

bool workspace_configuration::is_configuration_file(const utils::resource::resource_location& file) const
{
return is_config_file(file) || is_b4g_config_file(file);
return !m_location.empty() && (is_config_file(file) || is_b4g_config_file(file));
}

template<typename T>
Expand Down Expand Up @@ -553,7 +553,7 @@ bool workspace_configuration::settings_updated() const
}

utils::value_task<parse_config_file_result> workspace_configuration::parse_b4g_config_file(
const utils::resource::resource_location& cfg_file_rl)
utils::resource::resource_location cfg_file_rl)
{
// keep in sync with try_loading_alternative_configuration
const auto alternative_root =
Expand Down Expand Up @@ -737,13 +737,16 @@ void workspace_configuration::produce_diagnostics(
utils::value_task<parse_config_file_result> workspace_configuration::parse_configuration_file(
std::optional<utils::resource::resource_location> file)
{
if (!file.has_value() || is_config_file(*file))
co_return co_await load_and_process_config(m_config_diags);
if (!m_location.empty())
{
if (!file.has_value() || is_config_file(*file))
return load_and_process_config(m_config_diags);

if (is_b4g_config_file(*file))
co_return co_await parse_b4g_config_file(*file);
if (is_b4g_config_file(*file))
return parse_b4g_config_file(std::move(*file));
}

co_return parse_config_file_result::not_found;
return utils::value_task<parse_config_file_result>::from_value(parse_config_file_result::not_found);
}

utils::value_task<std::optional<std::vector<const processor_group*>>> workspace_configuration::refresh_libraries(
Expand Down Expand Up @@ -958,7 +961,7 @@ utils::value_task<utils::resource::resource_location> workspace_configuration::l
}
}

if (affiliation == regex_pgm)
if (affiliation == regex_pgm || m_location.empty())
co_return empty_alternative_cfg_root;

auto configuration_url = utils::resource::resource_location::replace_filename(rl, B4G_CONF_FILE);
Expand Down
2 changes: 1 addition & 1 deletion parser_library/src/workspaces/workspace_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ class workspace_configuration
bool is_b4g_config_file(const utils::resource::resource_location& file) const;

[[nodiscard]] utils::value_task<parse_config_file_result> parse_b4g_config_file(
const utils::resource::resource_location& cfg_file_rl);
utils::resource::resource_location cfg_file_rl);

[[nodiscard]] utils::value_task<parse_config_file_result> load_and_process_config(std::vector<diagnostic_s>& diags);

Expand Down
10 changes: 5 additions & 5 deletions parser_library/test/workspace/b4g_integration_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ using namespace hlasm_plugin::parser_library::workspaces;
using hlasm_plugin::utils::resource::resource_location;

namespace {
constexpr auto prepend_ws_loc = [](std::string path) {
constexpr auto prepend_ws_loc = [](std::string_view path) {
static const resource_location rl("scheme://ws/");
return resource_location::join(rl, path).lexically_normal();
};

const std::string empty_b4g_conf(R"({})");
const auto ws_rl = prepend_ws_loc("");
const auto proc_grps_rl = prepend_ws_loc(proc_grps_name.get_uri());
const auto pgm_conf_rl = prepend_ws_loc(pgm_conf_name.get_uri());
const auto proc_grps_rl = prepend_ws_loc(proc_grps_name);
const auto pgm_conf_rl = prepend_ws_loc(pgm_conf_name);
const auto b4g_conf_rl = prepend_ws_loc("SYS/SUB/ASMPGM/.bridge.json");
const auto pgm_a = prepend_ws_loc("SYS/SUB/ASMPGM/A");
const auto pgm_b = prepend_ws_loc("SYS/SUB/ASMPGM/B");
Expand Down Expand Up @@ -136,7 +136,7 @@ class workspace_test : public workspace
{
public:
workspace_test(file_manager& fm)
: workspace(ws_rl, "workspace_name", fm, m_config, m_global_settings)
: workspace(ws_rl, fm, m_config, m_global_settings)
{
open().run();
}
Expand Down Expand Up @@ -200,7 +200,7 @@ class pgm_conf_preference_helper
file_manager_impl_test fm;
lib_config config;
shared_json global_settings = make_empty_shared_json();
workspace ws = workspace(ws_rl, "workspace_name", fm, config, global_settings);
workspace ws = workspace(ws_rl, fm, config, global_settings);

pgm_conf_preference_helper()
{
Expand Down
28 changes: 14 additions & 14 deletions parser_library/test/workspace/diags_suppress_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ const auto file_loc = resource_location("a_file");
TEST(diags_suppress, no_suppress)
{
file_manager_impl fm;
fm.did_open_file(pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(proc_grps_name, 0, one_proc_grps);
fm.did_open_file(empty_pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(empty_proc_grps_name, 0, one_proc_grps);

fm.did_open_file(file_loc, 0, R"(
LR 1,
Expand All @@ -60,7 +60,7 @@ TEST(diags_suppress, no_suppress)
lib_config config;
shared_json global_settings = make_empty_shared_json();

workspace ws(fm, config, global_settings);
workspace ws(empty_ws, fm, config, global_settings);
ws.open().run();
run_if_valid(ws.did_open_file(file_loc));
parse_all_files(ws);
Expand All @@ -76,8 +76,8 @@ TEST(diags_suppress, do_suppress)
shared_json global_settings = make_empty_shared_json();

file_manager_impl fm;
fm.did_open_file(pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(proc_grps_name, 0, one_proc_grps);
fm.did_open_file(empty_pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(empty_proc_grps_name, 0, one_proc_grps);

fm.did_open_file(file_loc, 0, R"(
LR 1,
Expand All @@ -90,7 +90,7 @@ TEST(diags_suppress, do_suppress)

message_consumer_mock msg_consumer;

workspace ws(fm, config, global_settings);
workspace ws(empty_ws, fm, config, global_settings);
ws.set_message_consumer(&msg_consumer);
ws.open().run();
run_if_valid(ws.did_open_file(file_loc));
Expand All @@ -105,8 +105,8 @@ TEST(diags_suppress, do_suppress)
TEST(diags_suppress, pgm_supress_limit_changed)
{
file_manager_impl fm;
fm.did_open_file(pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(proc_grps_name, 0, one_proc_grps);
fm.did_open_file(empty_pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(empty_proc_grps_name, 0, one_proc_grps);

fm.did_open_file(file_loc, 0, R"(
LR 1,
Expand All @@ -120,7 +120,7 @@ TEST(diags_suppress, pgm_supress_limit_changed)
lib_config config;
shared_json global_settings = make_empty_shared_json();

workspace ws(fm, config, global_settings);
workspace ws(empty_ws, fm, config, global_settings);
ws.open().run();
run_if_valid(ws.did_open_file(file_loc));
parse_all_files(ws);
Expand All @@ -131,8 +131,8 @@ TEST(diags_suppress, pgm_supress_limit_changed)
std::string new_limit_str = R"("diagnosticsSuppressLimit":5,)";
document_change ch(range({ 0, 1 }, { 0, 1 }), new_limit_str.c_str(), new_limit_str.size());

fm.did_change_file(pgm_conf_name, 1, &ch, 1);
run_if_valid(ws.did_change_file(pgm_conf_name, file_content_state::changed_content));
fm.did_change_file(empty_pgm_conf_name, 1, &ch, 1);
run_if_valid(ws.did_change_file(empty_pgm_conf_name, file_content_state::changed_content));
parse_all_files(ws);

run_if_valid(ws.did_change_file(file_loc, file_content_state::changed_content));
Expand All @@ -146,8 +146,8 @@ TEST(diags_suppress, pgm_supress_limit_changed)
TEST(diags_suppress, mark_for_parsing_only)
{
file_manager_impl fm;
fm.did_open_file(pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(proc_grps_name, 0, one_proc_grps);
fm.did_open_file(empty_pgm_conf_name, 0, empty_pgm_conf);
fm.did_open_file(empty_proc_grps_name, 0, one_proc_grps);

fm.did_open_file(file_loc, 0, R"(
LR 1,
Expand All @@ -161,7 +161,7 @@ TEST(diags_suppress, mark_for_parsing_only)
auto config = lib_config::load_from_json(R"({"diagnosticsSuppressLimit":5})"_json);
shared_json global_settings = make_empty_shared_json();

workspace ws(fm, config, global_settings);
workspace ws(empty_ws, fm, config, global_settings);
ws.open().run();
run_if_valid(ws.did_open_file(file_loc));
// parsing not done yet
Expand Down
8 changes: 6 additions & 2 deletions parser_library/test/workspace/empty_configs.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@

#include <memory>
#include <string>
#include <string_view>

#include "nlohmann/json_fwd.hpp"
#include "utils/path.h"
#include "utils/resource_location.h"

inline const auto pgm_conf_name = hlasm_plugin::utils::resource::resource_location(".hlasmplugin/pgm_conf.json");
inline const auto proc_grps_name = hlasm_plugin::utils::resource::resource_location(".hlasmplugin/proc_grps.json");
constexpr const std::string_view pgm_conf_name(".hlasmplugin/pgm_conf.json");
constexpr const std::string_view proc_grps_name(".hlasmplugin/proc_grps.json");
inline const hlasm_plugin::utils::resource::resource_location empty_ws("ews:/");
inline const hlasm_plugin::utils::resource::resource_location empty_pgm_conf_name("ews:/.hlasmplugin/pgm_conf.json");
inline const hlasm_plugin::utils::resource::resource_location empty_proc_grps_name("ews:/.hlasmplugin/proc_grps.json");
inline const std::string empty_pgm_conf = R"({ "pgms": []})";
inline const std::string empty_proc_grps = R"({ "pgroups": []})";

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) 2023 Broadcom.
* The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Broadcom, Inc. - initial API and implementation
*/

#include <optional>
#include <string>

#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include "external_configuration_requests.h"

namespace {
class external_configuration_requests_mock : public hlasm_plugin::parser_library::external_configuration_requests
{
public:
MOCK_METHOD(void,
read_external_configuration,
(hlasm_plugin::parser_library::sequence<char> url,
hlasm_plugin::parser_library::workspace_manager_response<hlasm_plugin::parser_library::sequence<char>>
content),
(override));
};
} // namespace
21 changes: 9 additions & 12 deletions parser_library/test/workspace/instruction_sets_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,11 @@ std::string sam31_macro = R"( MACRO
&VAR SETA 2
MEND)";

const char* sam31_macro_path = is_windows() ? "lib\\SAM31" : "lib/SAM31";
const std::string hlasmplugin_folder = ".hlasmplugin";

const resource_location proc_grps_loc(hlasmplugin_folder + "/proc_grps.json");
const resource_location pgm_conf_loc(hlasmplugin_folder + "/pgm_conf.json");
const resource_location source_loc("source");
const resource_location ws_loc("ws:/");
const resource_location proc_grps_loc("ws:/.hlasmplugin/proc_grps.json");
const resource_location pgm_conf_loc("ws:/.hlasmplugin/pgm_conf.json");
const resource_location source_loc("ws:/source");
const resource_location sam31_macro_loc("ws:/lib/SAM31");

enum class file_manager_opt_variant
{
Expand All @@ -127,8 +126,6 @@ class file_manager_opt : public file_manager_impl
public:
file_manager_opt(file_manager_opt_variant variant)
{
resource_location sam31_macro_loc(sam31_macro_path);

did_open_file(proc_grps_loc, 1, get_proc_grp(variant));
did_open_file(pgm_conf_loc, 1, pgmconf_file);
did_open_file(source_loc, 1, source);
Expand All @@ -139,10 +136,10 @@ class file_manager_opt : public file_manager_impl
const hlasm_plugin::utils::resource::resource_location& location) const override
{
using hlasm_plugin::utils::value_task;
if (location == resource_location("lib/"))
if (location == resource_location("ws:/lib/"))
return value_task<list_directory_result>::from_value({
{
{ "SAM31", resource_location(sam31_macro_path) },
{ "SAM31", sam31_macro_loc },
},
hlasm_plugin::utils::path::list_directory_rc::done,
});
Expand Down Expand Up @@ -171,7 +168,7 @@ TEST_F(workspace_instruction_sets_test, changed_instr_set_370_Z10)
file_manager_opt file_manager(file_manager_opt_variant::optable_370);
lib_config config;
shared_json global_settings = make_empty_shared_json();
workspace ws(file_manager, config, global_settings);
workspace ws(ws_loc, file_manager, config, global_settings);
ws.open().run();

run_if_valid(ws.did_open_file(source_loc));
Expand All @@ -190,7 +187,7 @@ TEST_F(workspace_instruction_sets_test, changed_instr_set_Z10_370)
file_manager_opt file_manager(file_manager_opt_variant::optable_Z10);
lib_config config;
shared_json global_settings = make_empty_shared_json();
workspace ws(file_manager, config, global_settings);
workspace ws(ws_loc, file_manager, config, global_settings);
ws.open().run();

run_if_valid(ws.did_open_file(source_loc));
Expand Down
Loading

0 comments on commit 239bf21

Please sign in to comment.