Remove warnings and fixes for deprecated config (#7440)

Merge pull request 7440

docs/_docs/upgrading/3-to-4.md

@@ -156,3 +156,13 @@ Notes:
     ```
   * Vendors that provide a versioned Jekyll Environment Image (e.g. Docker Image, GitHub Pages, etc)
     will have to manually whitelist kramdown's extension gems in their distributions for Jekyll 4.0.
+
+## Deprecated Configuration Options
+
+Jekyll 4.0 has dropped support for all legacy configuration options that were deprecated over multiple
+releases in the previous series.
+
+To that end, we shall no longer output a deprecation warning when we encounter a legacy config key nor
+shall we gracefully assign their values to the newer counterparts. Depending on the key, it shall either
+be ignored or raise an `InvalidConfigurationError` error if the key is still valid but the associated
+value is not of the valid type.

features/markdown.feature

@@ -21,7 +21,7 @@ Feature: Markdown
     Given I have a configuration file with:
       | key      | value             |
       | paginate | 5                 |
-      | gems     | [jekyll-paginate] |
+      | plugins  | [jekyll-paginate] |
     And I have an "index.html" page that contains "Index - {% for post in paginator.posts %} {{ post.content }} {% endfor %}"
     And I have a _posts directory
     And I have the following post:

features/pagination.feature

@@ -7,7 +7,7 @@ Feature: Site pagination
     Given I have a configuration file with:
       | key      | value             |
       | paginate | <num>             |
-      | gems     | [jekyll-paginate] |
+      | plugins  | [jekyll-paginate] |
     And I have a _layouts directory
     And I have an "index.html" page that contains "{{ paginator.posts.size }}"
     And I have a _posts directory
@@ -35,7 +35,7 @@ Feature: Site pagination
       | paginate      | 1                              |
       | paginate_path | /blog/page-:num                |
       | permalink     | /blog/:year/:month/:day/:title |
-      | gems          | [jekyll-paginate]              |
+      | plugins       | [jekyll-paginate]              |
     And I have a blog directory
     And I have an "blog/index.html" page that contains "{{ paginator.posts.size }}"
     And I have a _posts directory
@@ -63,7 +63,7 @@ Feature: Site pagination
       | paginate      | 1                              |
       | paginate_path | /blog/page/:num                |
       | permalink     | /blog/:year/:month/:day/:title |
-      | gems          | [jekyll-paginate]              |
+      | plugins       | [jekyll-paginate]              |
     And I have a blog directory
     And I have an "blog/index.html" page that contains "{{ paginator.posts.size }}"
     And I have an "index.html" page that contains "Don't pick me!"

features/plugins.feature

@@ -4,7 +4,7 @@ Feature: Configuring and using plugins
 
   Scenario: Add a gem-based plugin
     Given I have an "index.html" file that contains "Whatever"
-    And I have a configuration file with "gems" set to "[jekyll_test_plugin]"
+    And I have a configuration file with "plugins" set to "[jekyll_test_plugin]"
     When I run jekyll build
     Then I should get a zero exit status
     And the _site directory should exist
@@ -15,7 +15,7 @@ Feature: Configuring and using plugins
     Given I have an "index.html" file that contains "Whatever"
     And I have a configuration file with:
       | key       | value                |
-      | gems      | [jekyll_test_plugin] |
+      | plugins   | [jekyll_test_plugin] |
       | whitelist | []                   |
     When I run jekyll build --safe
     Then I should get a zero exit status
@@ -27,7 +27,7 @@ Feature: Configuring and using plugins
     Given I have an "index.html" file that contains "Whatever"
     And I have a configuration file with:
       | key       | value                                              |
-      | gems      | [jekyll_test_plugin, jekyll_test_plugin_malicious] |
+      | plugins   | [jekyll_test_plugin, jekyll_test_plugin_malicious] |
       | whitelist | [jekyll_test_plugin]                               |
     When I run jekyll build --safe
     Then I should get a zero exit status

features/rendering.feature

@@ -98,7 +98,7 @@ Feature: Rendering
   Scenario: Don't place asset files in layout
     Given I have an "index.scss" page with layout "simple" that contains ".foo-bar { color:black; }"
     And I have an "index.coffee" page with layout "simple" that contains "whatever()"
-    And I have a configuration file with "gems" set to "[jekyll-coffeescript]"
+    And I have a configuration file with "plugins" set to "[jekyll-coffeescript]"
     And I have a simple layout that contains "{{ content }}Ahoy, indeed!"
     When I run jekyll build
     Then I should get a zero exit status
@@ -165,7 +165,7 @@ Feature: Rendering
 
   Scenario: Render liquid in CoffeeScript with jekyll-coffeescript enabled
     Given I have an "index.coffee" page with animal "cicada" that contains "hey='for {{page.animal}}'"
-    And I have a configuration file with "gems" set to "[jekyll-coffeescript]"
+    And I have a configuration file with "plugins" set to "[jekyll-coffeescript]"
     When I run jekyll build
     Then I should get a zero exit status
     And the _site directory should exist

jekyll.gemspec

@@ -48,7 +48,7 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency("safe_yaml",             "~> 1.0")
 
   s.post_install_message = <<~MSG
-    ----------------------------------------------------------------------------------
+    -------------------------------------------------------------------------------------
     This version of Jekyll comes with some major changes.
 
     Most notably:
@@ -59,6 +59,10 @@ Gem::Specification.new do |s|
       * Our `post_url` tag now comes with the `relative_url` filter incorporated into it.
         You shouldn't prepend `{{ site.baseurl }}` to `{% post_url 2019-03-27-hello %}`
         For further details: https://github.com/jekyll/jekyll/pull/7589
-    ----------------------------------------------------------------------------------
+
+      * Support for deprecated configuration options has been removed. We will no longer
+        output a warning and gracefully assign their values to the newer counterparts
+        internally.
+    -------------------------------------------------------------------------------------
   MSG
 end

lib/jekyll/configuration.rb

@@ -79,15 +79,11 @@ module Jekyll
 
     class << self
       # Static: Produce a Configuration ready for use in a Site.
-      # It takes the input, fills in the defaults where values do not
+      # It takes the input, fills in the defaults where values do not exist.
-      # exist, and patches common issues including migrating options for
-      # backwards compatiblity. Except where a key or value is being fixed,
-      # the user configuration will override the defaults.
       #
       # user_config - a Hash or Configuration of overrides.
       #
-      # Returns a Configuration filled with defaults and fixed for common
+      # Returns a Configuration filled with defaults.
-      # problems and backwards-compatibility.
       def from(user_config)
         Utils.deep_merge_hashes(DEFAULTS, Configuration[user_config].stringify_keys)
           .add_default_collections.add_default_excludes
@@ -132,8 +128,8 @@ module Jekyll
       when %r!\.ya?ml!i
         SafeYAML.load_file(filename) || {}
       else
-        raise ArgumentError, "No parser for '#{filename}' is available.
+        raise ArgumentError,
-          Use a .y(a)ml or .toml file instead."
+              "No parser for '#{filename}' is available. Use a .y(a)ml or .toml file instead."
       end
     end
 
@@ -169,7 +165,11 @@ module Jekyll
     def read_config_file(file)
       file = File.expand_path(file)
       next_config = safe_load_file(file)
-      check_config_is_hash!(next_config, file)
+
+      unless next_config.is_a?(Hash)
+        raise ArgumentError, "Configuration file: (INVALID) #{file}".yellow
+      end
+
       Jekyll.logger.info "Configuration file:", file
       next_config
     rescue SystemCallError
@@ -177,8 +177,7 @@ module Jekyll
         Jekyll.logger.warn "Configuration file:", "none"
         {}
       else
-        Jekyll.logger.error "Fatal:", "The configuration file '#{file}'
+        Jekyll.logger.error "Fatal:", "The configuration file '#{file}' could not be found."
-          could not be found."
         raise LoadError, "The Configuration file '#{file}' could not be found."
       end
     end
@@ -200,12 +199,11 @@ module Jekyll
           configuration = Utils.deep_merge_hashes(configuration, new_config)
         end
       rescue ArgumentError => e
-        Jekyll.logger.warn "WARNING:", "Error reading configuration. " \
+        Jekyll.logger.warn "WARNING:", "Error reading configuration. Using defaults (and options)."
-                     "Using defaults (and options)."
         warn e
       end
 
-      configuration.backwards_compatibilize.add_default_collections
+      configuration.validate.add_default_collections
     end
 
     # Public: Split a CSV string into an array containing its values
@@ -217,35 +215,18 @@ module Jekyll
       csv.split(",").map(&:strip)
     end
 
-    # Public: Ensure the proper options are set in the configuration to allow for
+    # Public: Ensure the proper options are set in the configuration
-    # backwards-compatibility with Jekyll pre-1.0
     #
-    # Returns the backwards-compatible configuration
+    # Returns the configuration Hash
-    def backwards_compatibilize
+    def validate
       config = clone
-      # Provide backwards-compatibility
+
-      check_auto(config)
-      check_server(config)
       check_plugins(config)
-
-      renamed_key "server_port", "port", config
-      renamed_key "gems", "plugins", config
-      renamed_key "layouts", "layouts_dir", config
-      renamed_key "data_source", "data_dir", config
-
-      check_pygments(config)
       check_include_exclude(config)
-      check_coderay(config)
-      check_maruku(config)
 
       config
     end
 
-    # DEPRECATED.
-    def fix_common_issues
-      self
-    end
-
     def add_default_collections
       config = clone
 
@@ -284,15 +265,6 @@ module Jekyll
       config
     end
 
-    def renamed_key(old, new, config)
-      if config.key?(old)
-        Jekyll::Deprecator.deprecation_message "The '#{old}' configuration" \
-          " option has been renamed to '#{new}'. Please update your config" \
-          " file accordingly."
-        config[new] = config.delete(old)
-      end
-    end
-
     private
 
     def style_to_permalink(permalink_style)
@@ -312,77 +284,13 @@ module Jekyll
       end
     end
 
-    # Private: Checks if a given config is a hash
-    #
-    # extracted_config - the value to check
-    # file - the file from which the config was extracted
-    #
-    # Raises an ArgumentError if given config is not a hash
-    def check_config_is_hash!(extracted_config, file)
-      unless extracted_config.is_a?(Hash)
-        raise ArgumentError, "Configuration file: (INVALID) #{file}".yellow
-      end
-    end
-
-    def check_auto(config)
-      if config.key?("auto") || config.key?("watch")
-        Jekyll::Deprecator.deprecation_message "Auto-regeneration can no longer" \
-                            " be set from your configuration file(s). Use the" \
-                            " --[no-]watch/-w command-line option instead."
-        config.delete("auto")
-        config.delete("watch")
-      end
-    end
-
-    def check_server(config)
-      if config.key?("server")
-        Jekyll::Deprecator.deprecation_message "The 'server' configuration option" \
-                            " is no longer accepted. Use the 'jekyll serve'" \
-                            " subcommand to serve your site with WEBrick."
-        config.delete("server")
-      end
-    end
-
-    def check_pygments(config)
-      if config.key?("pygments")
-        Jekyll::Deprecator.deprecation_message "The 'pygments' configuration option" \
-                            " has been renamed to 'highlighter'. Please update your" \
-                            " config file accordingly. The allowed values are 'rouge', " \
-                            "'pygments' or null."
-
-        config["highlighter"] = "pygments" if config["pygments"]
-        config.delete("pygments")
-      end
-    end
-
     def check_include_exclude(config)
       %w(include exclude).each do |option|
-        if config[option].is_a?(String)
+        next unless config.key?(option)
-          Jekyll::Deprecator.deprecation_message "The '#{option}' configuration option" \
+        next if config[option].is_a?(Array)
-            " must now be specified as an array, but you specified" \
-            " a string. For now, we've treated the string you provided" \
-            " as a list of comma-separated values."
-          config[option] = csv_to_array(config[option])
-        end
-        config[option]&.map!(&:to_s)
-      end
-    end
 
-    def check_coderay(config)
+        raise Jekyll::Errors::InvalidConfigurationError,
-      if (config["kramdown"] || {}).key?("use_coderay")
+              "'#{option}' should be set as an array, but was: #{config[option].inspect}."
-        Jekyll::Deprecator.deprecation_message "Please change 'use_coderay'" \
-          " to 'enable_coderay' in your configuration file."
-        config["kramdown"]["use_coderay"] = config["kramdown"].delete("enable_coderay")
-      end
-    end
-
-    def check_maruku(config)
-      if config.fetch("markdown", "kramdown").to_s.casecmp("maruku").zero?
-        Jekyll.logger.abort_with "Error:", "You're using the 'maruku' " \
-          "Markdown processor, which has been removed as of 3.0.0. " \
-          "We recommend you switch to Kramdown. To do this, replace " \
-          "`markdown: maruku` with `markdown: kramdown` in your " \
-          "`_config.yml` file."
       end
     end
 
@@ -391,17 +299,16 @@ module Jekyll
     # config - the config hash
     #
     # Raises a Jekyll::Errors::InvalidConfigurationError if the config `plugins`
-    # is a string
+    # is not an Array.
     def check_plugins(config)
-      if config.key?("plugins") && config["plugins"].is_a?(String)
+      return unless config.key?("plugins")
-        Jekyll.logger.error "Configuration Error:", "You specified the" \
+      return if config["plugins"].is_a?(Array)
-          " `plugins` config in your configuration file as a string, please" \
+
-          " use an array instead. If you wanted to set the directory of your" \
+      Jekyll.logger.error "'plugins' should be set as an array of gem-names, but was: " \
-          " plugins, use the config key `plugins_dir` instead."
+        "#{config["plugins"].inspect}. Use 'plugins_dir' instead to set the directory " \
+        "for your non-gemified Ruby plugins."
       raise Jekyll::Errors::InvalidConfigurationError,
-              "'plugins' should not be a string, but was: " \
+            "'plugins' should be set as an array, but was: #{config["plugins"].inspect}."
-              "#{config["plugins"].inspect}. Use 'plugins_dir' instead."
-      end
     end
   end
 end

test/test_configuration.rb

@@ -21,7 +21,7 @@ class TestConfiguration < JekyllUnitTest
     end
 
     should "return a valid Configuration instance" do
-      assert_instance_of Configuration, Configuration.from({}).fix_common_issues
+      assert_instance_of Configuration, Configuration.from({})
     end
 
     should "add default collections" do
@@ -92,15 +92,19 @@ class TestConfiguration < JekyllUnitTest
     should "only assign collections.posts.permalink if a permalink is specified" do
       result = Configuration[{ "permalink" => "pretty", "collections" => {} }]
         .add_default_collections
-      expected = { "posts" => {
+
+      expected = {
+        "posts" => {
           "output"    => true,
           "permalink" => "/:categories/:year/:month/:day/:title/",
-      } }
+        },
+      }
+
       assert_equal expected, result["collections"]
 
-      result = Configuration[{ "permalink" => nil, "collections" => {} }]
+      result   = Configuration[{ "permalink" => nil, "collections" => {} }].add_default_collections
-        .add_default_collections
       expected = { "posts" => { "output" => true } }
+
       assert_equal expected, result["collections"]
     end
 
@@ -120,6 +124,7 @@ class TestConfiguration < JekyllUnitTest
         :include   => [".htaccess"],
         :source    => "./",
       }]
+
       @string_keys = Configuration[{
         "markdown"  => "kramdown",
         "permalink" => "date",
@@ -128,13 +133,16 @@ class TestConfiguration < JekyllUnitTest
         "source"    => "./",
       }]
     end
+
     should "stringify symbol keys" do
       assert_equal @string_keys, @mixed_keys.stringify_keys
     end
+
     should "not mess with keys already strings" do
       assert_equal @string_keys, @string_keys.stringify_keys
     end
   end
+
   context "#config_files" do
     setup do
       @config = Configuration[{ "source" => source_dir }]
@@ -150,32 +158,39 @@ class TestConfiguration < JekyllUnitTest
       assert @config.config_files(@one_config_file).is_a?(Array)
       assert @config.config_files(@multiple_files).is_a?(Array)
     end
+
     should "return the default config path if no config files are specified" do
       assert_equal [source_dir("_config.yml")], @config.config_files(@no_override)
     end
+
     should "return .yaml if it exists but .yml does not" do
       allow(File).to receive(:exist?).with(source_dir("_config.yml")).and_return(false)
       allow(File).to receive(:exist?).with(source_dir("_config.yaml")).and_return(true)
       assert_equal [source_dir("_config.yaml")], @config.config_files(@no_override)
     end
+
     should "return .yml if both .yml and .yaml exist" do
       allow(File).to receive(:exist?).with(source_dir("_config.yml")).and_return(true)
       assert_equal [source_dir("_config.yml")], @config.config_files(@no_override)
     end
+
     should "return .toml if that exists" do
       allow(File).to receive(:exist?).with(source_dir("_config.yml")).and_return(false)
       allow(File).to receive(:exist?).with(source_dir("_config.yaml")).and_return(false)
       allow(File).to receive(:exist?).with(source_dir("_config.toml")).and_return(true)
       assert_equal [source_dir("_config.toml")], @config.config_files(@no_override)
     end
+
     should "return .yml if both .yml and .toml exist" do
       allow(File).to receive(:exist?).with(source_dir("_config.yml")).and_return(true)
       allow(File).to receive(:exist?).with(source_dir("_config.toml")).and_return(true)
       assert_equal [source_dir("_config.yml")], @config.config_files(@no_override)
     end
+
     should "return the config if given one config file" do
       assert_equal %w(config.yml), @config.config_files(@one_config_file)
     end
+
     should "return an array of the config files if given many config files" do
       assert_equal(
         %w(config/site.yml config/deploy.toml configuration.yml),
@@ -204,77 +219,59 @@ class TestConfiguration < JekyllUnitTest
 
     should "continue to read config files if one is empty" do
       allow(SafeYAML).to receive(:load_file).with(File.expand_path("empty.yml")).and_return(false)
-      allow(SafeYAML)
+      allow(SafeYAML).to receive(:load_file).with(File.expand_path("not_empty.yml")).and_return(
-        .to receive(:load_file)
+        "foo" => "bar"
-        .with(File.expand_path("not_empty.yml"))
+      )
-        .and_return("foo" => "bar", "include" => "", "exclude" => "")
       Jekyll.logger.log_level = :warn
-      read_config = @config.read_config_files(["empty.yml", "not_empty.yml"])
+      read_config = @config.read_config_files(%w(empty.yml not_empty.yml))
       Jekyll.logger.log_level = :info
       assert_equal "bar", read_config["foo"]
     end
   end
-  context "#backwards_compatibilize" do
+
+  context "#validate" do
     setup do
       @config = Configuration[{
         "auto"        => true,
         "watch"       => true,
         "server"      => true,
-        "exclude"     => "READ-ME.md, Gemfile,CONTRIBUTING.hello.markdown",
-        "include"     => "STOP_THE_PRESSES.txt,.heloses, .git",
         "pygments"    => true,
         "layouts"     => true,
         "data_source" => true,
         "gems"        => [],
       }]
     end
-    should "unset 'auto' and 'watch'" do
+
-      assert @config.key?("auto")
+    should "raise an error if `exclude` key is a string" do
-      assert @config.key?("watch")
+      config = Configuration[{ "exclude" => "READ-ME.md, Gemfile,CONTRIBUTING.hello.markdown" }]
-      assert !@config.backwards_compatibilize.key?("auto")
+      assert_raises(Jekyll::Errors::InvalidConfigurationError) { config.validate }
-      assert !@config.backwards_compatibilize.key?("watch")
     end
-    should "unset 'server'" do
+
-      assert @config.key?("server")
+    should "raise an error if `include` key is a string" do
-      assert !@config.backwards_compatibilize.key?("server")
+      config = Configuration[{ "include" => "STOP_THE_PRESSES.txt,.heloses, .git" }]
-    end
+      assert_raises(Jekyll::Errors::InvalidConfigurationError) { config.validate }
-    should "transform string exclude into an array" do
-      assert @config.key?("exclude")
-      assert @config.backwards_compatibilize.key?("exclude")
-      expected = %w(READ-ME.md Gemfile CONTRIBUTING.hello.markdown)
-      assert_equal expected, @config.backwards_compatibilize["exclude"]
-    end
-    should "transform string include into an array" do
-      assert @config.key?("include")
-      assert @config.backwards_compatibilize.key?("include")
-      expected = %w(STOP_THE_PRESSES.txt .heloses .git)
-      assert_equal expected, @config.backwards_compatibilize["include"]
-    end
-    should "set highlighter to pygments" do
-      assert @config.key?("pygments")
-      assert !@config.backwards_compatibilize.key?("pygments")
-      assert_equal "pygments", @config.backwards_compatibilize["highlighter"]
-    end
-    should "adjust directory names" do
-      assert @config.key?("layouts")
-      assert !@config.backwards_compatibilize.key?("layouts")
-      assert @config.backwards_compatibilize["layouts_dir"]
-      assert @config.key?("data_source")
-      assert !@config.backwards_compatibilize.key?("data_source")
-      assert @config.backwards_compatibilize["data_dir"]
     end
+
     should "raise an error if `plugins` key is a string" do
       config = Configuration[{ "plugins" => "_plugin" }]
-      assert_raises Jekyll::Errors::InvalidConfigurationError do
+      assert_raises(Jekyll::Errors::InvalidConfigurationError) { config.validate }
-        config.backwards_compatibilize
     end
-    end
+
-    should "set the `gems` config to `plugins`" do
+    should "not rename configuration keys" do
+      assert @config.key?("layouts")
+      assert @config.validate.key?("layouts")
+      refute @config.validate.key?("layouts_dir")
+
+      assert @config.key?("data_source")
+      assert @config.validate.key?("data_source")
+      refute @config.validate.key?("data_dir")
+
       assert @config.key?("gems")
-      assert !@config.backwards_compatibilize["gems"]
+      assert @config.validate.key?("gems")
-      assert @config.backwards_compatibilize["plugins"]
+      refute @config.validate.key?("plugins")
     end
   end
+
   context "loading configuration" do
     setup do
       @path = source_dir("_config.yml")
@@ -331,6 +328,7 @@ class TestConfiguration < JekyllUnitTest
       # as opposed to: assert_equal ':foo', SafeYAML.load(':foo')
     end
   end
+
   context "loading config from external file" do
     setup do
       @paths = {