form_with/fields: Don't output ids by default

Continuing 67f81cc where we decided not to output ids
by default in the new form helpers.

Went with @dhh's suggestion of just requiring ids on
fields being labelled:
https://github.com/rails/rails/issues/25197#issuecomment-231797117

Seems okay enough.

3 files changed, 43 insertions, 6 deletions

diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb
index e7ea267211..55389950ed 100644
--- a/actionview/lib/action_view/helpers/form_helper.rb
+++ b/actionview/lib/action_view/helpers/form_helper.rb
@@ -595,6 +595,16 @@ module ActionView
       #
       # Where <tt>@document = Document.find(params[:id])</tt>.
       #
+      # When using labels +form_with+ requires setting the id on the field being
+      # labelled:
+      #
+      #   <%= form_with(model: @post) do |form| %>
+      #     <%= form.label :title %>
+      #     <%= form.text_field :title, id: :post_title %>
+      #   <% end %>
+      #
+      # See +label+ for more on how the +for+ attribute is derived.
+      #
       # === Mixing with other form helpers
       #
       # While +form_with+ uses a FormBuilder object it's possible to mix and
@@ -690,6 +700,8 @@ module ActionView
       #     form_with(**options.merge(builder: LabellingFormBuilder), &block)
       #   end
       def form_with(model: nil, scope: nil, url: nil, format: nil, **options)
+        options[:skip_default_ids] = true
+
         if model
           url ||= polymorphic_path(model, format: format)
 
@@ -986,6 +998,16 @@ module ActionView
       # or model is yielded, so any generated field names are prefixed with
       # either the passed scope or the scope inferred from the <tt>:model</tt>.
       #
+      # When using labels +fields+ requires setting the id on the field being
+      # labelled:
+      #
+      #   <%= fields :comment do |fields| %>
+      #     <%= fields.label :body %>
+      #     <%= fields.text_field :body, id: :comment_body %>
+      #   <% end %>
+      #
+      # See +label+ for more on how the +for+ attribute is derived.
+      #
       # === Mixing with other form helpers
       #
       # While +form_with+ uses a FormBuilder object it's possible to mix and
@@ -1003,7 +1025,8 @@ module ActionView
       # to work with an object as a base, like
       # FormOptionHelper#collection_select and DateHelper#datetime_select.
       def fields(scope = nil, model: nil, **options, &block)
-        # TODO: Remove when ids and classes are no longer output by default.
+        options[:skip_default_ids] = true
+
         if model
           scope ||= model_name_from_record_or_class(model).param_key
         end
@@ -1469,7 +1492,7 @@ module ActionView
       private
         def html_options_for_form_with(url_for_options = nil, model = nil, html: {}, local: false,
           skip_enforcing_utf8: false, **options)
-          html_options = options.except(:index, :include_id, :builder).merge(html)
+          html_options = options.except(:index, :include_id, :skip_default_ids, :builder).merge(html)
           html_options[:method] ||= :patch if model.respond_to?(:persisted?) && model.persisted?
           html_options[:enforce_utf8] = !skip_enforcing_utf8
 
@@ -1603,7 +1626,7 @@ module ActionView
       def initialize(object_name, object, template, options)
         @nested_child_index = {}
         @object_name, @object, @template, @options = object_name, object, template, options
-        @default_options = @options ? @options.slice(:index, :namespace) : {}
+        @default_options = @options ? @options.slice(:index, :namespace, :skip_default_ids) : {}
 
         convert_to_legacy_options(@options)
 
@@ -1910,6 +1933,8 @@ module ActionView
 
       # See the docs for the <tt>ActionView::FormHelper.fields</tt> helper method.
       def fields(scope = nil, model: nil, **options, &block)
+        options[:skip_default_ids] = true
+
         convert_to_legacy_options(options)
 
         fields_for(scope || model, model, **options, &block)
diff --git a/actionview/lib/action_view/helpers/tags/base.rb b/actionview/lib/action_view/helpers/tags/base.rb
index cf8a6d6028..b8c446cbed 100644
--- a/actionview/lib/action_view/helpers/tags/base.rb
+++ b/actionview/lib/action_view/helpers/tags/base.rb
@@ -13,6 +13,7 @@ module ActionView
 
           @object_name.sub!(/\[\]$/, "") || @object_name.sub!(/\[\]\]$/, "]")
           @object = retrieve_object(options.delete(:object))
+          @skip_default_ids = options.delete(:skip_default_ids)
           @options = options
           @auto_index = Regexp.last_match ? retrieve_autoindex(Regexp.last_match.pre_match) : nil
         end
@@ -81,9 +82,12 @@ module ActionView
           def add_default_name_and_id(options)
             index = name_and_id_index(options)
             options["name"] = options.fetch("name") { tag_name(options["multiple"], index) }
-            options["id"] = options.fetch("id") { tag_id(index) }
-            if namespace = options.delete("namespace")
-              options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace
+
+            unless skip_default_ids?
+              options["id"] = options.fetch("id") { tag_id(index) }
+              if namespace = options.delete("namespace")
+                options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace
+              end
             end
           end
 
@@ -154,6 +158,10 @@ module ActionView
           def name_and_id_index(options)
             options.key?("index") ? options.delete("index") || "" : @auto_index
           end
+
+          def skip_default_ids?
+            @skip_default_ids
+          end
       end
     end
   end
diff --git a/actionview/lib/action_view/helpers/tags/label.rb b/actionview/lib/action_view/helpers/tags/label.rb
index b31d5fda66..cab15ae201 100644
--- a/actionview/lib/action_view/helpers/tags/label.rb
+++ b/actionview/lib/action_view/helpers/tags/label.rb
@@ -73,6 +73,10 @@ module ActionView
           def render_component(builder)
             builder.translation
           end
+
+          def skip_default_ids?
+            false # The id is used as the `for` attribute.
+          end
       end
     end
   end