config: some implementation refactoring

Author: aviatesk

src/config.jl

@@ -58,6 +58,42 @@ end
 on_difference(callback, old_val, new_val, path::Tuple{Vararg{Symbol}}) =
     old_val !== new_val ? callback(old_val, new_val, path) : old_val
 
+function parse_config_dict(config_dict::AbstractDict{String}, filepath::Union{Nothing,AbstractString} = nothing)
+    try
+        return Configurations.from_dict(JETLSConfig, config_dict)
+    catch e
+        # TODO: remove this when Configurations.jl support to report
+        #       full path of unknown key.
+        if e isa Configurations.InvalidKeyError
+            unknown_keys = collect_unmatched_keys(to_untyped_config_dict(config_dict))
+            if !isempty(unknown_keys)
+                if isnothing(filepath)
+                    return unmatched_keys_in_config_file_msg(unknown_keys)
+                else
+                    return unmatched_keys_in_config_file_msg(filepath, unknown_keys)
+                end
+            end
+        elseif e isa DiagnosticConfigError
+            if isnothing(filepath)
+                return "Invalid diagnostic configuration: $(e.msg)"
+            else
+                return """
+                Invalid diagnostic configuration in $filepath:
+                $(e.msg)
+                """
+            end
+        end
+        if isnothing(filepath)
+            return "Failed to parse LSP configuration: $(e)"
+        else
+            return """
+            Failed to load configuration file at $filepath:
+            $(e)
+            """
+        end
+    end
+end
+
 """
     merge_setting(base::T, overlay::T) where {T<:ConfigSection} -> T
 
@@ -69,36 +105,37 @@ merge_setting(base::T, overlay::T) where {T<:ConfigSection} =
 
 # TODO: remove this.
 #       (now this is used for `collect_unmatched_keys` only. see that's comment)
-const ConfigDict = Base.PersistentDict{String, Any}
-to_config_dict(dict::AbstractDict) = ConfigDict((k => (v isa AbstractDict ? to_config_dict(v) : v) for (k, v) in dict)...)
+const UntypedConfigDict = Base.PersistentDict{String, Any}
+to_untyped_config_dict(dict::AbstractDict) =
+    UntypedConfigDict((k => (v isa AbstractDict ? to_untyped_config_dict(v) : v) for (k, v) in dict)...)
 
-const DEFAULT_CONFIG_DICT = to_config_dict(Configurations.to_dict(DEFAULT_CONFIG))
+const DEFAULT_UNTYPED_CONFIG_DICT = to_untyped_config_dict(Configurations.to_dict(DEFAULT_CONFIG))
 
 """
-    collect_unmatched_keys(this::ConfigDict, ref::ConfigDict) -> Vector{Vector{String}}
+    collect_unmatched_keys(this::UntypedConfigDict, ref::UntypedConfigDict) -> Vector{Vector{String}}
 
 Traverses the keys of `this` and returns a list of key paths that are not present in `ref`.
 Note that this function does *not* perform deep structural comparison for keys whose values are dictionaries.
 
 # Examples
 ```julia-repl
 julia> collect_unmatched_keys(
-            ConfigDict("key1" => ConfigDict("key2" => 0, "key3"  => 0, "key4"  => 0)),
-            ConfigDict("key1" => ConfigDict("key2" => 0, "diff1" => 0, "diff2" => 0))
+            UntypedConfigDict("key1" => UntypedConfigDict("key2" => 0, "key3"  => 0, "key4"  => 0)),
+            UntypedConfigDict("key1" => UntypedConfigDict("key2" => 0, "diff1" => 0, "diff2" => 0))
         )
 2-element Vector{Vector{String}}:
  ["key1", "key3"]
  ["key1", "key4"]
 
 julia> collect_unmatched_keys(
-            ConfigDict("key1" => 0, "key2" => 0),
-            ConfigDict("key1" => 1, "key2" => 1)
+            UntypedConfigDict("key1" => 0, "key2" => 0),
+            UntypedConfigDict("key1" => 1, "key2" => 1)
         )
 Vector{String}[]
 
 julia> collect_unmatched_keys(
-           ConfigDict("key1" => ConfigDict("key2" => 0, "key3" => 0)),
-           ConfigDict("diff" => ConfigDict("diff" => 0, "key3" => 0))
+           UntypedConfigDict("key1" => UntypedConfigDict("key2" => 0, "key3" => 0)),
+           UntypedConfigDict("diff" => UntypedConfigDict("diff" => 0, "key3" => 0))
         )
 1-element Vector{Vector{String}}:
  ["key1"]
@@ -107,15 +144,15 @@ julia> collect_unmatched_keys(
 TODO: remove this. This is a temporary workaround to report unknown keys in the config file
       until Configurations.jl supports reporting full path of unknown keys.
 """
-function collect_unmatched_keys(this::ConfigDict, ref::ConfigDict=DEFAULT_CONFIG_DICT)
+function collect_unmatched_keys(this::UntypedConfigDict, ref::UntypedConfigDict=DEFAULT_UNTYPED_CONFIG_DICT)
     unknown_keys = Vector{String}[]
     collect_unmatched_keys!(unknown_keys, this, ref, String[])
     return unknown_keys
 end
 
 function collect_unmatched_keys!(
         unknown_keys::Vector{Vector{String}},
-        this::ConfigDict, ref::ConfigDict, key_path::Vector{String}
+        this::UntypedConfigDict, ref::UntypedConfigDict, key_path::Vector{String}
     )
     for (k, v) in this
         current_path = [key_path; k]
@@ -144,9 +181,9 @@ so `config` is guaranteed to not be `nothing`.
 Base.@constprop :aggressive function get_config(manager::ConfigManager, key_path::Symbol...)
     data = load(manager)
     config = if is_static_setting(key_path...)
-        getobjpath(merge_setting(DEFAULT_CONFIG, data.static_settings), key_path...)
+        getobjpath(data.__filled_static_settings__, key_path...)
     else
-        getobjpath(merge_setting(DEFAULT_CONFIG, get_settings(data)), key_path...)
+        getobjpath(data.__filled_settings__, key_path...)
     end
     @assert !isnothing(config) "Invalid default configuration values"
     return config

src/diagnostics.jl

@@ -153,7 +153,7 @@ function calculate_match_specificity(
     return specificity
 end
 
-function _apply_diagnostic_config(diagnostic::Diagnostic, diagnostic_config::DiagnosticConfig)
+function _apply_diagnostic_config(diagnostic::Diagnostic, manager::ConfigManager)
     code = diagnostic.code
     if !(code isa String)
         if JETLS_DEV_MODE
@@ -171,7 +171,8 @@ function _apply_diagnostic_config(diagnostic::Diagnostic, diagnostic_config::Dia
         return diagnostic
     end
 
-    patterns = @something diagnostic_config.patterns return diagnostic
+    patterns = get_config(manager, :diagnostic, :patterns)
+    isempty(patterns) && return diagnostic
 
     message = diagnostic.message
     severity = nothing
@@ -199,14 +200,10 @@ function _apply_diagnostic_config(diagnostic::Diagnostic, diagnostic_config::Dia
 end
 
 function apply_diagnostic_config!(diagnostics::Vector{Diagnostic}, manager::ConfigManager)
-    settings = get_settings(load(manager))
-    diagnostic_config = @something settings.diagnostic default_config(DiagnosticConfig)
-    if diagnostic_config.enabled === false
-        return empty!(diagnostics)
-    end
+    get_config(manager, :diagnostic, :enabled) || return empty!(diagnostics)
     i = 1
     while i <= length(diagnostics)
-        applied = _apply_diagnostic_config(diagnostics[i], diagnostic_config)
+        applied = _apply_diagnostic_config(diagnostics[i], manager)
         if applied === missing
             deleteat!(diagnostics, i)
             continue

src/did-change-watched-files.jl

@@ -65,32 +65,11 @@ function load_file_config!(callback, server::Server, filepath::AbstractString;
         parsed = TOML.tryparsefile(filepath)
         parsed isa TOML.ParserError && return old_data, nothing
 
-        new_file_config = try
-            Configurations.from_dict(JETLSConfig, parsed)
-        catch e
-            # TODO: remove this when Configurations.jl support to report
-            #       full path of unknown key.
-            if e isa Configurations.InvalidKeyError
-                config_dict = to_config_dict(parsed)
-                unknown_keys = collect_unmatched_keys(config_dict)
-                if !isempty(unknown_keys)
-                    show_error_message(server, unmatched_keys_in_config_file_msg(filepath, unknown_keys))
-                    return old_data, nothing
-                end
-            elseif e isa DiagnosticConfigError
-                show_error_message(server, """
-                    Invalid diagnostic configuration in $filepath:
-                    $(e.msg)
-                    """)
-                return old_data, nothing
-            end
-            show_error_message(server, """
-                Failed to load configuration file at $filepath:
-                $(e)
-                """)
+        new_file_config = parse_config_dict(parsed, filepath)
+        if new_file_config isa AbstractString
+            show_error_message(server, new_file_config)
             return old_data, nothing
         end
-
         new_data = ConfigManagerData(old_data;
             file_config=new_file_config,
             file_config_path=filepath

src/types.jl

@@ -359,13 +359,13 @@ end
 
 const DIAGNOSTIC_SOURCE = "JETLS"
 
-const VALID_DIAGNOSTIC_CATEGORIES = Set{String}([
+const VALID_DIAGNOSTIC_CATEGORIES = Set{String}((
     "syntax",
     "lowering",
     "toplevel",
     "inference",
     "testrunner",
-])
+))
 
 const SYNTAX_DIAGNOSTIC_CODE = "syntax/parse-error"
 const LOWERING_UNUSED_ARGUMENT_CODE = "lowering/unused-argument"
@@ -409,7 +409,7 @@ Base.convert(::Type{DiagnosticPattern}, x::AbstractDict{String}) =
 end
 
 is_static_setting(::Type{DiagnosticConfig}, ::Symbol) = false
-default_config(::Type{DiagnosticConfig}) = DiagnosticConfig(true, nothing)
+default_config(::Type{DiagnosticConfig}) = DiagnosticConfig(true, DiagnosticPattern[])
 
 # configuration item for test purpose
 @option struct InternalConfig <: ConfigSection
@@ -456,6 +456,8 @@ struct ConfigManagerData
     lsp_config::JETLSConfig
     file_config_path::Union{Nothing,String}
     __settings__::JETLSConfig
+    __filled_static_settings__::JETLSConfig
+    __filled_settings__::JETLSConfig
     function ConfigManagerData(
             static_settings::JETLSConfig,
             file_config::JETLSConfig,
@@ -470,10 +472,14 @@ struct ConfigManagerData
         #    - Limited to project root scope only
         #    - Takes precedence since clients don't properly support
         #      hierarchical configuration via scopeUri
-        settings = DEFAULT_CONFIG
-        settings = merge_setting(settings, lsp_config)
-        settings = merge_setting(settings, file_config)
-        return new(static_settings, file_config, lsp_config, file_config_path, settings)
+        __settings__ = DEFAULT_CONFIG
+        __settings__ = merge_setting(__settings__, lsp_config)
+        __settings__ = merge_setting(__settings__, file_config)
+        # Create setting structs without `nothing` values for use by `get_config`
+        __filled_static_settings__ = merge_setting(DEFAULT_CONFIG, static_settings)
+        __filled_settings__ = merge_setting(DEFAULT_CONFIG, __settings__)
+        return new(static_settings, file_config, lsp_config, file_config_path,
+            __settings__, __filled_static_settings__, __filled_settings__)
     end
 end
 

src/workspace-configuration.jl

@@ -75,23 +75,11 @@ function store_lsp_config!(tracker::ConfigChangeTracker, server::Server, @nospec
         return delete_lsp_config!(tracker, server)
     end
     store!(server.state.config_manager) do old_data::ConfigManagerData
-        new_lsp_config = try
-            Configurations.from_dict(JETLSConfig, config_dict)
-        catch e
-            if e isa Configurations.InvalidKeyError
-                unknown_keys = collect_unmatched_keys(to_config_dict(config_dict))
-                if !isempty(unknown_keys)
-                    show_error_message(server, unmatched_keys_in_lsp_config_msg(unknown_keys))
-                    return old_data, nothing
-                end
-            elseif e isa DiagnosticConfigError
-                show_error_message(server, "Invalid diagnostic configuration: $(e.msg)")
-                return old_data, nothing
-            end
-            show_error_message(server, "Failed to parse LSP configuration: $(e)")
+        new_lsp_config = parse_config_dict(config_dict)
+        if new_lsp_config isa AbstractString
+            show_error_message(server, new_lsp_config)
             return old_data, nothing
         end
-
         new_data = ConfigManagerData(old_data; lsp_config=new_lsp_config)
         on_difference(tracker, get_settings(old_data), get_settings(new_data))
         return new_data, nothing

test/test_config.jl

@@ -69,23 +69,23 @@ using JETLS
     end
 end
 
-@testset "ConfigDict utilities" begin
-    TEST_DICT = JETLS.ConfigDict(
+@testset "UntypedConfigDict utilities" begin
+    TEST_DICT = JETLS.UntypedConfigDict(
         "test_key1" => "test_value1",
-        "test_key2" => JETLS.ConfigDict(
+        "test_key2" => JETLS.UntypedConfigDict(
             "nested_key1" => "nested_value1",
-            "nested_key2" => JETLS.ConfigDict(
+            "nested_key2" => JETLS.UntypedConfigDict(
                 "deep_nested_key1" => "deep_nested_value1",
                 "deep_nested_key2" => "deep_nested_value2"
             )
         )
     )
 
-    TEST_DICT_DIFFERENT_KEY = JETLS.ConfigDict(
+    TEST_DICT_DIFFERENT_KEY = JETLS.UntypedConfigDict(
         "diffname_1" => "test_value1",
-        "test_key2" => JETLS.ConfigDict(
+        "test_key2" => JETLS.UntypedConfigDict(
             "nested_key1" => "nested_value1",
-            "diffname_2" => JETLS.ConfigDict(
+            "diffname_2" => JETLS.UntypedConfigDict(
                 "deep_nested_key1" => "deep_nested_value1",
                 "diffname_3" => "deep_nested_value2"
             )
@@ -102,9 +102,9 @@ end
 
         @test isempty(JETLS.collect_unmatched_keys(TEST_DICT, TEST_DICT))
 
-        # single-arg version should use DEFAULT_CONFIG_DICT
+        # single-arg version should use DEFAULT_UNTYPED_CONFIG_DICT
         @test JETLS.collect_unmatched_keys(TEST_DICT_DIFFERENT_KEY) ==
-              JETLS.collect_unmatched_keys(TEST_DICT_DIFFERENT_KEY, JETLS.DEFAULT_CONFIG_DICT)
+              JETLS.collect_unmatched_keys(TEST_DICT_DIFFERENT_KEY, JETLS.DEFAULT_UNTYPED_CONFIG_DICT)
     end
 end