Resolved unused variable warnings

[iainbeeston]

If you run avo with ruby warnings enabled you will see a lot of assigned but unused variable - property warnings. This is happening because the dynamically generated initializer always includes a property local variable even if it's never used.

To fix this I've removed that variable entirely (and accessed the values we need directly from properties). This is the only place that the properties variable was accessed so I've turned that into a method instead (it was too complex to access this directly every time it's needed).

Fixes #13

[iainbeeston]

CI is failing but it looks like it's never passed on this repo?

[Paul-Bob]

This is happening because the dynamically generated initializer always includes a property local variable even if it's never used.

Hi @iainbeeston thanks for looking into this.

property is used when generate_initializer_coerce_property is called.

Can this warning be a false positive?

[iainbeeston]

Sorry I should have included some examples so you can see the code generated before and after (it's very hard to see what's happening with the code generation and the warning doesn't happen every time).

If I run rails with warnings enabled before, prop initialiser generates dynamic methods for lots of classes but (for example) this is the code generated for Avo::Views::ResourceShowComponent:

  # frozen_string_literal: true
  def initialize(resource: nil, reflection: nil, parent_resource: nil, parent_record: nil, resource_panel: nil, actions: PropInitializer::Null)
    properties = self.class.prop_initializer_properties.properties_index
    # resource
    property = properties[:resource]
    @resource = resource
    # reflection
    property = properties[:reflection]
    @reflection = reflection
    # parent_resource
    property = properties[:parent_resource]
    @parent_resource = parent_resource
    # parent_record
    property = properties[:parent_record]
    @parent_record = parent_record
    # resource_panel
    property = properties[:resource_panel]
    @resource_panel = resource_panel
    # actions
    property = properties[:actions]
    if PropInitializer::Null == actions
      actions = property.default_value
    end
    @actions = actions
    after_initialize if respond_to?(:after_initialize)
  end
  
  def to_h
    {
      resource: @resource,
      reflection: @reflection,
      parent_resource: @parent_resource,
      parent_record: @parent_record,
      resource_panel: @resource_panel,
      actions: @actions,
    }
  end
  
  public
  
  def actions
    value = @actions
    value
  end

The generated code using this PR:

  # frozen_string_literal: true
  def initialize(resource: nil, reflection: nil, parent_resource: nil, parent_record: nil, resource_panel: nil, actions: PropInitializer::Null)
    # resource
    @resource = resource
    # reflection
    @reflection = reflection
    # parent_resource
    @parent_resource = parent_resource
    # parent_record
    @parent_record = parent_record
    # resource_panel
    @resource_panel = resource_panel
    # actions
    if PropInitializer::Null == actions
      actions = properties[:actions].default_value
    end
    @actions = actions
    after_initialize if respond_to?(:after_initialize)
  end
  
  def properties
    self.class.prop_initializer_properties.properties_index
  end
  
  def to_h
    {
      resource: @resource,
      reflection: @reflection,
      parent_resource: @parent_resource,
      parent_record: @parent_record,
      resource_panel: @resource_panel,
      actions: @actions,
    }
  end
  
  public

  def actions
    value = @actions
    value
  end

You can see in the first example (which uses prop_initializer version 0.2.0) properties is assigned but never used, but with this PR we never assign property and instead call a method to get properties when they are needed.

[iainbeeston]

The warning is coming from the prop initializer in these classes in avo 3.17.6:

• Avo::AlertComponent
• Avo::Views::ResourceShowComponent
• Avo::Items::VisibleItemsComponent
• Avo::Items::PanelComponent
• Avo::Views::ResourceEditComponent
• Avo::PaginatorComponent
• Avo::Index::FieldWrapperComponent
• Avo::Index::TableRowComponent
• Avo::Index::ResourceTableComponent
• Avo::Index::ResourceControlsComponent
• Avo::Views::ResourceIndexComponent
• Avo::TurboFrameWrapperComponent
• Avo::SidebarProfileComponent
• Avo::Sidebar::HeadingComponent
• Avo::Sidebar::LinkComponent
• Avo::Sidebar::BaseItemComponent
• Avo::Pro::GlobalSearchComponent
• Avo::ButtonComponent
• Avo::AssetManager::JavascriptComponent
• Avo::AssetManager::StylesheetComponent
• Avo::LoadingComponent
• Avo::PanelNameComponent
• Avo::ProfilePhotoComponent
• Avo::PanelHeaderComponent
• Avo::CoverPhotoComponent
• Avo::PanelComponent

In all of those cases, there is a line like property = properties[:foo] in the initializer which is never used (like the example above)