codeclimate-duplication icon indicating copy to clipboard operation
codeclimate-duplication copied to clipboard

Rails params whitelist marked

Open ivdma opened this issue 9 years ago • 2 comments

Two different Rails params whitelists are marked as duplicates with a threshold of 23.

# Controller A
params.require(:model_a).permit(:name, :description, :automatic, other_model_ids: []).tap do |model_a_params|
      model_a_params[:settings] = params[:model_a][:settings]
end

# Controller B
params[:model_b].permit(:happened_at, :other_model_id, :some_type, some_other_models_ids: [])
                         .tap { |whitelisted| whitelisted[:reason] = params[:model_b][:reason] }

There are some similarities, but this shouldn't be a duplication error.

ivdma avatar Mar 04 '16 11:03 ivdma

Hi @ivdma,

Right now the duplication algorithm effectively compares syntax nodes based on both what kind of expression they represent, as well as the contents of those expressions. When the kinds of the expressions are the same, but the contents vary (which looks to be the case here), the engine considers that "similar" duplication to distinguish it from "identical" duplication.

The detection of "similar" duplication like your case is one that, while it can turn up helpful results in many cases, is unfortunately also prone to these kinds of false-positives that are easy to distinguish as a human but not easy to make a decision about within an algorithm. We are continuing to investigate ways of improving the algorithm for these kinds of cases, but need to be careful that in avoiding false-positives we don't swing too far and stop returning useful instances of similar code. So for the time being we don't consider this behavior a bug since this is the expected behavior of Flay, the underlying tool this engine uses for detecting duplication.

There are several workarounds available to you in the meantime:

  1. You can ignore particular issues using their fingerprints.

  2. You can tune the mass threshold of duplicate code that the engine will report. This will effect both similar & identical issues. (Ruby's default mass threshold is 18.)

    engines:
      duplication:
        enabled: true
        config:
          ruby:
            mass_threshold: 25
    
  3. If you would prefer not to see reports of similar code at all, you can disable that check entirely. If you do this, the engine will only report instances of code it sees as identical.

    engines:
      duplication:
        enabled: true
        checks:
          Similar code:
            enabled: false
    

wfleming avatar Mar 31 '16 19:03 wfleming

See #190 for my proposed solution to this.

zenspider avatar Jun 14 '17 23:06 zenspider