Merge pull request #2578 from abicky/resolve-2572

Improve unused parameter warning messages from sections used at least once

lib/fluent/config/element.rb

@@ -31,7 +31,7 @@ def initialize(name, arg, attrs, elements, unused = nil)
         @unused = unused || attrs.keys
         @v1_config = false
         @corresponding_proxies = [] # some plugins use flat parameters, e.g. in_http doesn't provide <format> section for parser.
-        @unused_in = false # if this element is not used in plugins, correspoing plugin name and parent element name is set, e.g. [source, plugin class].
+        @unused_in = nil # if this element is not used in plugins, correspoing plugin name and parent element name is set, e.g. [source, plugin class].
 
         # it's global logger, not plugin logger: deprecated message should be global warning, not plugin level.
         @logger = defined?($log) ? $log : nil
@@ -110,13 +110,13 @@ def each_element(*names, &block)
       end
 
       def has_key?(key)
-        @unused_in = false # some sections, e.g. <store> in copy, is not defined by config_section so clear unused flag for better warning message in check_not_fetched.
+        @unused_in = [] # some sections, e.g. <store> in copy, is not defined by config_section so clear unused flag for better warning message in check_not_fetched.
         @unused.delete(key)
         super
       end
 
       def [](key)
-        @unused_in = false # ditto
+        @unused_in = [] # ditto
         @unused.delete(key)
 
         if RESERVED_PARAMETERS.include?(key) && !has_key?(key) && has_key?(RESERVED_PARAMETERS_COMPAT[key])

lib/fluent/config/section.rb

@@ -207,8 +207,11 @@ def self.check_unused_section(proxy, conf, plugin_class)
         elems = conf.respond_to?(:elements) ? conf.elements : []
         elems.each { |e|
           next if plugin_class.nil? && Fluent::Config::V1Parser::ELEM_SYMBOLS.include?(e.name) # skip pre-defined non-plugin elements because it doens't have proxy section
+          next if e.unused_in && e.unused_in.empty? # the section is used at least once
 
-          unless proxy.sections.any? { |name, subproxy| e.name == subproxy.name.to_s || e.name == subproxy.alias.to_s }
+          if proxy.sections.any? { |name, subproxy| e.name == subproxy.name.to_s || e.name == subproxy.alias.to_s }
+            e.unused_in = []
+          else
             parent_name = if conf.arg.empty?
                             conf.name
                           else

test/config/test_configurable.rb

@@ -152,7 +152,7 @@ class Example0
     config_param :arrayvalue, :array
   end
 
-  class Example1
+  class ExampleWithAlias
     include Fluent::Configurable
 
     config_param :name, :string, alias: :fullname
@@ -166,22 +166,7 @@ def get_all
     end
   end
 
-  class Example3
-    include Fluent::Configurable
-
-    config_param :age, :integer, default: 10
-
-    config_section :appendix, required: true, multi: false, final: true do
-      config_param :type, :string
-      config_param :name, :string, default: "x"
-    end
-
-    def get_all
-      [@name, @detail]
-    end
-  end
-
-  class Example5
+  class ExampleWithSecret
     include Fluent::Configurable
 
     config_param :normal_param, :string
@@ -193,17 +178,42 @@ class Example5
     end
   end
 
-  class Example6
+  class ExampleWithDefaultHashAndArray
     include Fluent::Configurable
     config_param :obj1, :hash, default: {}
     config_param :obj2, :array, default: []
   end
 
-  class Example7
+  class ExampleWithSkipAccessor
     include Fluent::Configurable
     config_param :name, :string, default: 'example7', skip_accessor: true
   end
 
+  class ExampleWithCustomSection
+    include Fluent::Configurable
+    config_param :name_param, :string
+    config_section :normal_section  do
+      config_param :normal_section_param, :string
+    end
+
+    class CustomSection
+      include Fluent::Configurable
+      config_param :custom_section_param, :string
+    end
+
+    class AnotherElement
+      include Fluent::Configurable
+    end
+
+    def configure(conf)
+      super
+      conf.elements.each do |e|
+        next if e.name != 'custom_section'
+        CustomSection.new.configure(e)
+      end
+    end
+  end
+
   module Overwrite
     class Base
       include Fluent::Configurable
@@ -564,20 +574,20 @@ class TestConfigurable < ::Test::Unit::TestCase
 
         sub_test_case 'default values should be duplicated before touched in plugin code' do
           test 'default object should be dupped for cases configured twice' do
-            x6a = ConfigurableSpec::Example6.new
+            x6a = ConfigurableSpec::ExampleWithDefaultHashAndArray.new
             assert_nothing_raised { x6a.configure(config_element("")) }
             assert_equal({}, x6a.obj1)
             assert_equal([], x6a.obj2)
 
-            x6b = ConfigurableSpec::Example6.new
+            x6b = ConfigurableSpec::ExampleWithDefaultHashAndArray.new
             assert_nothing_raised { x6b.configure(config_element("")) }
             assert_equal({}, x6b.obj1)
             assert_equal([], x6b.obj2)
 
             assert { x6a.obj1.object_id != x6b.obj1.object_id }
             assert { x6a.obj2.object_id != x6b.obj2.object_id }
 
-            x6c = ConfigurableSpec::Example6.new
+            x6c = ConfigurableSpec::ExampleWithDefaultHashAndArray.new
             assert_nothing_raised { x6c.configure(config_element("")) }
             assert_equal({}, x6c.obj1)
             assert_equal([], x6c.obj2)
@@ -1184,7 +1194,7 @@ class TestConfigurable < ::Test::Unit::TestCase
     sub_test_case 'class defined with config_param/config_section having :alias' do
       sub_test_case '#initialize' do
         test 'does not create methods for alias' do
-          ex1 = ConfigurableSpec::Example1.new
+          ex1 = ConfigurableSpec::ExampleWithAlias.new
           assert_nothing_raised { ex1.name }
           assert_raise(NoMethodError) { ex1.fullname }
           assert_nothing_raised { ex1.bool }
@@ -1196,7 +1206,7 @@ class TestConfigurable < ::Test::Unit::TestCase
 
       sub_test_case '#configure' do
         test 'provides accessible data for alias attribute keys' do
-          ex1 = ConfigurableSpec::Example1.new
+          ex1 = ConfigurableSpec::ExampleWithAlias.new
           conf = config_element('ROOT', '', {
                                   "fullname" => "foo bar",
                                   "bool" => false
@@ -1288,7 +1298,7 @@ class TestConfigurable < ::Test::Unit::TestCase
                                  'secret_param' => 'secret'
                                },
                                [config_element('section', '', {'normal_param2' => 'normal', 'secret_param2' => 'secret'} )])
-        @example = ConfigurableSpec::Example5.new
+        @example = ConfigurableSpec::ExampleWithSecret.new
         @example.configure(@conf)
       end
 
@@ -1311,20 +1321,6 @@ class TestConfigurable < ::Test::Unit::TestCase
         }
       end
 
-      test 'get plugin name when found unknown section' do
-        @conf = config_element('ROOT', '',
-                               {
-                                 'normal_param' => 'normal',
-                                 'secret_param' => 'secret'
-                               },
-                               [config_element('unknown', '', {'normal_param2' => 'normal', 'secret_param2' => 'secret'} )])
-        @example = ConfigurableSpec::Example5.new
-        @example.configure(@conf)
-        @conf.elements.each { |e|
-          assert_equal(['ROOT', nil], e.unused_in)
-        }
-      end
-
       def assert_secret_param(key, value)
         case key
         when 'normal_param', 'normal_param2'
@@ -1335,9 +1331,51 @@ def assert_secret_param(key, value)
       end
     end
 
+    sub_test_case 'unused section' do
+      test 'get plugin name when found unknown section' do
+        conf = config_element('ROOT', '',
+                              {
+                                'name_param' => 'name',
+                              },
+                              [config_element('unknown', '', {'name_param' => 'normal'} )])
+        example = ConfigurableSpec::ExampleWithCustomSection.new
+        example.configure(conf)
+        conf.elements.each { |e|
+          assert_equal(['ROOT', nil], e.unused_in)
+        }
+      end
+
+      test 'get an empty array when the section is defined without using config_section' do
+        conf = config_element('ROOT', '',
+                              {
+                                'name_param' => 'name',
+                              },
+                              [config_element('custom_section', '', {'custom_section_param' => 'custom'} )])
+        example = ConfigurableSpec::ExampleWithCustomSection.new
+        example.configure(conf)
+        conf.elements.each { |e|
+          assert_equal([], e.unused_in)
+        }
+      end
+
+      test 'get an empty array when the configuration is used in another element without any sections' do
+        conf = config_element('ROOT', '',
+                              {
+                                'name_param' => 'name',
+                              },
+                              [config_element('normal_section', '', {'normal_section_param' => 'normal'} )])
+        example = ConfigurableSpec::ExampleWithCustomSection.new
+        example.configure(conf)
+        ConfigurableSpec::ExampleWithCustomSection::AnotherElement.new.configure(conf)
+        conf.elements.each { |e|
+          assert_equal([], e.unused_in)
+        }
+      end
+    end
+
     sub_test_case ':skip_accessor option' do
       test 'it does not create accessor methods for parameters' do
-        @example = ConfigurableSpec::Example7.new
+        @example = ConfigurableSpec::ExampleWithSkipAccessor.new
         @example.configure(config_element('ROOT'))
         assert_equal 'example7', @example.instance_variable_get(:@name)
         assert_raise NoMethodError.new("undefined method `name' for #{@example}") do