fix: File extensions are now ignored when macro_extensions option i…

…s not provided in proc_grps.json

README.md

@@ -219,6 +219,7 @@ The `alwaysRecognize` option in `pgm_conf.json` has been deprecated in favor of
 
 `proc_grps.json` can include an optional parameter `macro_extensions` which contains a list of extensions that are used to identify files with macro definitions.
 The options can be specified both at the top level of the file, which provides the default list for all libraries in all process groups, and at the level of individual library definitions, which override the default from the top level.
+If the `macro_extensions` parameter is not provided or is empty the language server ignores file extensions. Warning messages are produced when conflicting names are detected.
 
 For example, with the extension `.hlasm`, a user can add the macro `MAC` to his source code even if it is in a file called `MAC.hlasm`.
 

clients/vscode-hlasmplugin/CHANGELOG.md

@@ -23,6 +23,7 @@
 #### Changed
 - Macro label is the preferred go to definition target unless the request is made from the label itself
 - Library contents are now shared between processor groups
+- File extensions are now ignored when `macro_extensions` option is not provided in proc_grps.json
 
 ## [1.5.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.4.0...1.5.0) (2022-11-02)
 

parser_library/src/diagnostic.cpp

@@ -2660,30 +2660,22 @@ diagnostic_s diagnostic_s::error_L0002(
         concat("Unable to load library: ", lib_loc.to_presentable(), ". Error: The path does not point to directory."));
 }
 
-diagnostic_s diagnostic_s::warning_L0003(
-    const utils::resource::resource_location& config_loc, const utils::resource::resource_location& lib_loc)
-{
-    return diagnostic_s(config_loc.get_uri(),
-        {},
-        diagnostic_severity::warning,
-        "L0003",
-        concat("Macros from library '",
-            lib_loc.to_presentable(),
-            "' were selected by a deprecated mechanism to specify file extensions (alwaysRecognize in pgm_conf.json)."),
-        {},
-        diagnostic_tag::none);
-}
-
 diagnostic_s diagnostic_s::warning_L0004(const utils::resource::resource_location& config_loc,
     const utils::resource::resource_location& lib_loc,
-    std::string_view macro_name)
+    std::string_view macro_name,
+    bool has_extensions)
 {
     return diagnostic_s(config_loc.get_uri(),
         {},
         diagnostic_severity::warning,
         "L0004",
-        concat(
-            "Library '", lib_loc.to_presentable(), "' contains multiple definitions of the macro '", macro_name, "'."),
+        concat("Library '",
+            lib_loc.to_presentable(),
+            "' contains conflicting macro definitions (",
+            macro_name,
+            "). Consider ",
+            has_extensions ? std::string_view("changing") : std::string_view("adding"),
+            " 'macro_extensions' parameter."),
         {},
         diagnostic_tag::none);
 }

parser_library/src/diagnostic.h

@@ -866,12 +866,12 @@ class diagnostic_s
     static diagnostic_s error_L0002(
         const utils::resource::resource_location& config_loc, const utils::resource::resource_location& lib_loc);
 
-    static diagnostic_s warning_L0003(
-        const utils::resource::resource_location& config_loc, const utils::resource::resource_location& lib_loc);
+    // warning_L0003 - removed
 
     static diagnostic_s warning_L0004(const utils::resource::resource_location& config_loc,
         const utils::resource::resource_location& lib_loc,
-        std::string_view macro_name);
+        std::string_view macro_name,
+        bool has_extensions);
 
     static diagnostic_s warning_L0005(
         const utils::resource::resource_location& config_loc, std::string_view pattern, size_t limit);

parser_library/src/workspaces/library_local.cpp

@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <locale>
+#include <utility>
 
 #include "utils/path.h"
 #include "utils/platform.h"
@@ -24,19 +25,12 @@
 namespace hlasm_plugin::parser_library::workspaces {
 
 namespace {
-void adjust_extensions_vector(std::vector<std::string>& extensions, bool extensions_from_deprecated_source)
+void adjust_extensions_vector(std::vector<std::string>& extensions)
 {
-    bool contains_empty = false;
     for (auto& ext : extensions)
-    {
-        if (ext.empty())
-            contains_empty = true;
-        else
-        {
-            if (ext[0] != '.')
-                ext.insert(0, 1, '.');
-        }
-    }
+        if (!ext.empty() && ext[0] != '.')
+            ext.insert(0, 1, '.');
+
     // from longest to shortest, then lexicographically
     std::sort(extensions.begin(), extensions.end(), [](const std::string& l, const std::string& r) {
         if (l.size() > r.size())
@@ -45,8 +39,6 @@ void adjust_extensions_vector(std::vector<std::string>& extensions, bool extensi
             return false;
         return l < r;
     });
-    if (extensions_from_deprecated_source && !contains_empty)
-        extensions.emplace_back(); // alwaysRecognize always implied accepting files without an extension
     extensions.erase(std::unique(extensions.begin(), extensions.end()), extensions.end());
 }
 } // namespace
@@ -59,11 +51,10 @@ library_local::library_local(file_manager& file_manager,
     , m_lib_loc(std::move(lib_loc))
     , m_extensions(std::move(options.extensions))
     , m_optional(options.optional_library)
-    , m_extensions_from_deprecated_source(options.extensions_from_deprecated_source)
     , m_proc_grps_loc(std::move(proc_grps_loc))
 {
     if (m_extensions.size())
-        adjust_extensions_vector(m_extensions, m_extensions_from_deprecated_source);
+        adjust_extensions_vector(m_extensions);
 }
 
 library_local::library_local(library_local&& l) noexcept
@@ -72,7 +63,6 @@ library_local::library_local(library_local&& l) noexcept
     , m_files_collection(l.m_files_collection.exchange(nullptr))
     , m_extensions(std::move(l.m_extensions))
     , m_optional(l.m_optional)
-    , m_extensions_from_deprecated_source(l.m_extensions_from_deprecated_source)
     , m_proc_grps_loc(std::move(l.m_proc_grps_loc))
 {}
 
@@ -138,40 +128,55 @@ library_local::files_collection_t library_local::load_files()
             break;
     }
 
-    bool extension_removed = false;
+    size_t conflict_count = 0;
+    std::string file_name_conflicts;
+    const auto add_conflict = [&conflict_count, &file_name_conflicts](std::string_view file_name) {
+        constexpr const size_t max_conflict_count = 3;
+        if (conflict_count < max_conflict_count)
+        {
+            if (conflict_count)
+                file_name_conflicts.append(", ");
+            file_name_conflicts.append(file_name);
+        }
+        else if (conflict_count == max_conflict_count)
+            file_name_conflicts.append(" and others");
+        ++conflict_count;
+    };
+
     for (auto& [file, rl] : files_list)
     {
         if (m_extensions.empty())
         {
-            new_files.try_emplace(context::to_upper_copy(file), std::move(rl));
-            continue;
+            // ".hidden" is not an extension ------v
+            if (auto off = file.find_first_of('.', 1); off != std::string::npos)
+                file.erase(off);
         }
+        else if (auto ext = std::find_if(
+                     m_extensions.begin(), m_extensions.end(), [&f = file](const auto& e) { return f.ends_with(e); });
+                 ext != m_extensions.end())
 
-        for (const auto& extension : m_extensions)
-        {
-            std::string_view filename(file);
-
-            if (filename.size() <= extension.size())
-                continue;
-
-            if (filename.substr(filename.size() - extension.size()) != extension)
-                continue;
-            filename.remove_suffix(extension.size());
+            file.erase(file.size() - ext->size());
+        else
+            continue;
 
-            const auto [_, inserted] =
-                new_files.try_emplace(context::to_upper_copy(std::string(filename)), std::move(rl));
-            // TODO: the stored value is a full path, yet we try to interpret it as a relative one later on
-            if (!inserted)
-                new_diags.push_back(diagnostic_s::warning_L0004(
-                    m_proc_grps_loc, m_lib_loc, context::to_upper_copy(std::string(filename))));
+        context::to_upper(file);
 
-            if (extension.size())
-                extension_removed = true;
-            break;
+        if (auto [it, inserted] = new_files.try_emplace(std::move(file), std::move(rl)); !inserted)
+        {
+            // file, rl was not moved
+            add_conflict(file);
+
+            // keep shortest (i.e. without extension for compatibility) or lexicographically smaller
+            const std::string_view rl_uri(rl.get_uri());
+            const std::string_view old_uri(it->second.get_uri());
+            if (std::pair(rl_uri.size(), rl_uri) < std::pair(old_uri.size(), old_uri))
+                it->second = std::move(rl);
         }
     }
-    if (extension_removed && m_extensions_from_deprecated_source)
-        new_diags.push_back(diagnostic_s::warning_L0003(m_proc_grps_loc, m_lib_loc));
+
+    if (conflict_count > 0)
+        new_diags.push_back(
+            diagnostic_s::warning_L0004(m_proc_grps_loc, m_lib_loc, file_name_conflicts, !m_extensions.empty()));
 
     m_files_collection.store(new_state);
 

parser_library/src/workspaces/library_local.h

@@ -36,7 +36,6 @@ namespace hlasm_plugin::parser_library::workspaces {
 struct library_local_options
 {
     std::vector<std::string> extensions;
-    bool extensions_from_deprecated_source = false;
     bool optional_library = false;
 
 #ifdef __cpp_lib_three_way_comparison
@@ -49,10 +48,6 @@ struct library_local_options
             return true;
         if (extensions > o.extensions)
             return false;
-        if (extensions_from_deprecated_source < o.extensions_from_deprecated_source)
-            return true;
-        if (extensions_from_deprecated_source > o.extensions_from_deprecated_source)
-            return false;
         if (optional_library < o.optional_library)
             return true;
         return false;
@@ -118,7 +113,6 @@ class library_local final : public library
     atomic_files_collection_t m_files_collection;
     std::vector<std::string> m_extensions;
     bool m_optional = false;
-    bool m_extensions_from_deprecated_source = false;
     utils::resource::resource_location m_proc_grps_loc;
 
     files_collection_t load_files();

parser_library/src/workspaces/workspace_configuration.cpp

@@ -71,29 +71,6 @@ utils::resource::resource_location transform_to_resource_location(
     return rl.lexically_normal();
 }
 
-std::vector<std::string> get_macro_extensions_compatibility_list(std::span<const std::string> always_recognize)
-{
-    // Extract extension list for compatibility reasons
-    std::vector<std::string> macro_extensions_compatibility_list;
-    for (const auto& wildcard : always_recognize)
-    {
-        std::string_view wc(wildcard);
-        if (const auto ext_pattern = wc.rfind("*."); ext_pattern != std::string_view::npos)
-        {
-            wc.remove_prefix(ext_pattern + 1);
-            macro_extensions_compatibility_list.emplace_back(wc);
-        }
-    }
-
-    std::sort(macro_extensions_compatibility_list.begin(), macro_extensions_compatibility_list.end());
-
-    macro_extensions_compatibility_list.erase(
-        std::unique(macro_extensions_compatibility_list.begin(), macro_extensions_compatibility_list.end()),
-        macro_extensions_compatibility_list.end());
-
-    return macro_extensions_compatibility_list;
-}
-
 std::optional<std::string> substitute_home_directory(std::string p)
 {
     if (!p.starts_with("~"))
@@ -107,9 +84,8 @@ std::optional<std::string> substitute_home_directory(std::string p)
     return utils::path::join(homedir, std::move(p).substr(skip)).string();
 }
 
-library_local_options get_library_local_options(const config::library& lib,
-    std::span<const std::string> fallback_macro_extensions,
-    std::span<const std::string> always_recognize)
+library_local_options get_library_local_options(
+    const config::library& lib, std::span<const std::string> fallback_macro_extensions)
 {
     library_local_options opts;
 
@@ -118,12 +94,6 @@ library_local_options get_library_local_options(const config::library& lib,
         opts.extensions = lib.macro_extensions;
     else if (!fallback_macro_extensions.empty())
         opts.extensions = std::vector(fallback_macro_extensions.begin(), fallback_macro_extensions.end());
-    else if (auto macro_extensions_compatibility_list = get_macro_extensions_compatibility_list(always_recognize);
-             !macro_extensions_compatibility_list.empty())
-    {
-        opts.extensions = macro_extensions_compatibility_list;
-        opts.extensions_from_deprecated_source = true;
-    }
 
     return opts;
 }
@@ -292,7 +262,6 @@ std::shared_ptr<library> workspace_configuration::get_local_library(
 
 void workspace_configuration::process_processor_group(const config::processor_group& pg,
     std::span<const std::string> fallback_macro_extensions,
-    std::span<const std::string> always_recognize,
     const utils::resource::resource_location& alternative_root,
     std::vector<diagnostic_s>& diags)
 {
@@ -312,7 +281,7 @@ void workspace_configuration::process_processor_group(const config::processor_gr
             continue;
         }
 
-        auto lib_local_opts = get_library_local_options(lib, fallback_macro_extensions, always_recognize);
+        auto lib_local_opts = get_library_local_options(lib, fallback_macro_extensions);
         auto rl = transform_to_resource_location(*lib_path, root);
         rl.join(""); // Ensure that this is a directory
 
@@ -332,15 +301,14 @@ void workspace_configuration::process_processor_group(const config::processor_gr
 void workspace_configuration::process_processor_group_and_cleanup_libraries(
     std::span<const config::processor_group> pgs,
     std::span<const std::string> fallback_macro_extensions,
-    std::span<const std::string> always_recognize,
     const utils::resource::resource_location& alternative_root,
     std::vector<diagnostic_s>& diags)
 {
     for (auto& [_, l] : m_libraries)
         l.second = false; // mark
 
     for (const auto& pg : pgs)
-        process_processor_group(pg, fallback_macro_extensions, always_recognize, alternative_root, diags);
+        process_processor_group(pg, fallback_macro_extensions, alternative_root, diags);
 
     std::erase_if(m_libraries, [](const auto& kv) { return !kv.second.second; }); // sweep
 }
@@ -396,11 +364,8 @@ parse_config_file_result workspace_configuration::load_and_process_config(std::v
     config::pgm_conf pgm_config;
     const auto pgm_conf_loaded = load_pgm_config(pgm_config, utilized_settings_values, diags);
 
-    process_processor_group_and_cleanup_libraries(proc_groups.pgroups,
-        proc_groups.macro_extensions,
-        pgm_config.always_recognize,
-        utils::resource::resource_location(),
-        diags);
+    process_processor_group_and_cleanup_libraries(
+        proc_groups.pgroups, proc_groups.macro_extensions, utils::resource::resource_location(), diags);
 
     if (pgm_conf_loaded != parse_config_file_result::parsed)
     {
@@ -546,7 +511,7 @@ parse_config_file_result workspace_configuration::parse_b4g_config_file(
     }
 
     process_processor_group_and_cleanup_libraries(
-        m_proc_grps_source.pgroups, m_proc_grps_source.macro_extensions, {}, alternative_root, conf.diags);
+        m_proc_grps_source.pgroups, m_proc_grps_source.macro_extensions, alternative_root, conf.diags);
 
     for (const auto& [name, details] : conf.config.value().files)
     {

parser_library/src/workspaces/workspace_configuration.h

@@ -227,13 +227,11 @@ class workspace_configuration
 
     void process_processor_group(const config::processor_group& pg,
         std::span<const std::string> fallback_macro_extensions,
-        std::span<const std::string> always_recognize,
         const utils::resource::resource_location& alternative_root,
         std::vector<diagnostic_s>& diags);
 
     void process_processor_group_and_cleanup_libraries(std::span<const config::processor_group> pgs,
         std::span<const std::string> fallback_macro_extensions,
-        std::span<const std::string> always_recognize,
         const utils::resource::resource_location& alternative_root,
         std::vector<diagnostic_s>& diags);