Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapter for gems #1

Open
makaroni4 opened this issue Oct 29, 2012 · 8 comments
Open

Adapter for gems #1

makaroni4 opened this issue Oct 29, 2012 · 8 comments
Labels

Comments

@makaroni4
Copy link
Member

brakeman @lest
flay/flog @Proghat
railsbp @releu
reek @makaroni4

@makaroni4
Copy link
Member Author

So here is a simple script for reek:

require 'reek'

file = File.read 'filename.rb'

examiner = Reek::Examiner.new file

examiner.smells.each do |smell|
  puts "#{smell.class}\t#{smell.subclass}\t#{smell.message}\t#{smell.location['context']}\t#{smell.location['lines']}"
end

For each smell the most important params are:

  • class
  • subclass
  • message
  • location (array of line numbers)

Seems like we need not only write adapters but aggregate all the codesmells from all gems to add refactor_url to each of them. These codesmells will be hardcoded in our gem, lets call it cc-middleman 😄

@lest
Copy link

lest commented Oct 31, 2012

@makaroni4 Awesome! Could you please provide an example of param values?

@makaroni4
Copy link
Member Author

classes:
[Reek::SmellWarning]

subclasses:
["DuplicateMethodCall", "NestedIterators", "ControlParameter", "UncommunicativeVariableName", "RepeatedConditional", "TooManyStatements", "TooManyMethods", "UncommunicativeModuleName", "UncommunicativeParameterName", "FeatureEnvy", "UtilityFunction", "IrresponsibleModule"]

messages:
["calls can(:manage, :all) twice", "calls site_admin.permissions 7 times", "calls site_admin.permissions.joins(:section) 6 times", "calls site_admin.site_id 7 times", "calls user.id 6 times", "contains iterators nested 2 deep", "is controlled by argument user", "has the variable name 'c'", "tests (self.site and self.board_rubric) at least 4 times", "calls self.board_rubric 4 times", "calls self.site twice", "calls self.board_rubric twice", "calls r.board_types twice", "has approx 9 statements", "has the variable name 'r'", "has the variable name 't'", "calls self.board_type twice", "calls self.site 3 times", "calls self.board_global_rubric twice", "calls self.user 4 times", "calls self.user.fio twice", "calls self.counts twice", "calls self.counts.site(site) twice", "has at least 28 methods", "has the name 'Catalog2'", "calls a.present? twice", "has the variable name 'a'", "calls self.galleries twice", "calls self[:logo_alt] twice", "has the parameter name 'c'", "calls self[:meta_title] twice", "calls @cached_ratings["#{options[:year]}-#{options[:month]}"] 3 times", "calls Time.now twice", "calls options[:month] 8 times", "calls options[:update] twice", "calls options[:year] 8 times", "calls self.ratings twice", "has approx 14 statements", "refers to options more than self", "has the variable name 'k'", "has the variable name 'v'", "has approx 6 statements", "has the variable name 'i'", "calls catalog.id twice", "calls parent.parent_id twice", "calls rubric.catalogs twice", "has approx 13 statements", "has the variable name 'parent2'", "calls root_rubric.title twice", "has approx 11 statements", "has approx 7 statements", "calls x.rating twice", "calls y.rating twice", "has the variable name 'x'", "has the variable name 'y'", "has the variable name 'd'", "calls id twice", "calls param twice", "doesn't depend on instance state", "refers to r more than self", "calls rubrics twice", "calls site_id_was twice", "calls (host + "/catalog/") twice", "calls self.rubrics twice", "tests current at least 4 times", "calls ADS[self.id] twice", "calls self.id twice", "calls current.counts twice", "calls current.counts.site(site) twice", "refers to count more than self", "refers to current more than self", "calls count.counter twice", "calls count.site twice", "calls current.parent twice", "calls self.counts.each twice", "calls self.parent 3 times", "calls self.parent_id twice", "calls self.user 3 times", "calls self.comment 3 times", "calls self.comment twice", "calls self[:level] twice", "calls self.comment.comments twice", "has the variable name 'p'", "calls self[:position] twice", "calls comment.user 3 times", "calls doc.object_author twice", "has approx 12 statements", "calls self.position twice", "has the variable name 's'", "calls ((Date.today - Date.today.mday.days) + 1.day) twice", "calls (Date.today - Date.today.mday.days) twice", "calls 1.day twice", "calls Date.today 4 times", "calls Date.today.mday twice", "calls Date.today.mday.days twice", "calls self.global_rubric twice", "calls self.rubric twice", "calls Rails.env twice", "calls self.id.to_s twice", "calls self.param 3 times", "calls self.param.present? twice", "calls self.rubric 3 times", "tests self.rubric.present? at least 3 times", "has approx 8 statements", "calls self.rubric.present? twice", "calls ((Date.today - Date.today.mday.days) + 1.day) 4 times", "calls (Date.today - Date.today.mday.days) 4 times", "calls 1.day 5 times", "calls 1.month twice", "calls Date.today 10 times", "calls Date.today.mday 4 times", "calls Date.today.mday.days 4 times", "calls send(association) 3 times", "calls self.photo_global_rubric twice", "calls self.photos twice", "calls self.photos 4 times", "calls self.photos.main twice", "calls doc.annotation twice", "calls doc.title 3 times", "calls site twice", "has no descriptive comment", "has the variable name 'user1'", "has the variable name 'user2'", "calls help 3 times", "calls self.main_photo 5 times", "has approx 15 statements", "calls self.news_rubric twice"]

location context (location lines is just array of integers):
["Atm", "Board", "Board#board_rubric_id=", "Board#counters_dec", "Board#counters_inc", "Board#self.generate_board_types", "Board#self.generate_rubric_select", "Board#set_global_board_rubric", "Board#site_id=", "Board#url", "Board#user_name", "BoardGlobalRubric#counters_dec", "BoardGlobalRubric#counters_inc"]

@makaroni4
Copy link
Member Author

Seems like we don't need class because it is always Reek::SmellWarning

@makaroni4 makaroni4 reopened this Oct 31, 2012
@makaroni4
Copy link
Member Author

So I see the list of params like this:

Codesmell:

  • name
  • location
  • lines
  • message
  • price
  • refactor_url

@lest
Copy link

lest commented Nov 1, 2012

Here is brakeman json output: https://www.refheap.com/paste/701a97d125f5bab90e3c96198

All this information could be also extracted using the following ruby script:

require 'brakeman'
require 'brakeman/scanner'

options = Brakeman.set_options app_path: '../brakeman/test/apps/rails3.2'
scanner = Brakeman::Scanner.new options
tracker = scanner.process
tracker.run_checks

%w(model_warnings controller_warnings template_warnings warnings).each do |type|
  tracker.checks.send(type).each do |warning|
    p warning.check  
    p warning.confidence
    puts "#{warning.file}:#{warning.line}"
    p warning.class
    p warning.method
    p warning.warning_set
    p warning.warning_type
    p warning.called_from
    p warning.template
  end
end

PS. I don't like brakeman codebase – it smells a lot for me. I'd consider using standard json output and sending it to our app as is. In that case our app will be responsible to extract useful info from json and save it to the database.

@dzhlobo
Copy link

dzhlobo commented Nov 5, 2012

Here is how we can use flog

require 'flog'

flogger = Flog.new(all: true, parser: Ruby19Parser)
flogger.flog 'filename.rb'

flogger.each_by_score do |method, score, call_list|
  puts "#{method} => #{score.round(2)}"
end

Script's output looks like

Flog#flog => 59.83
Flog::parse_options => 59.33
Flog::load_plugins => 32.76
Flog#none => 32.7
Flog#report => 23.1
main#none => 5.5

where main#none is no class (no our class, we don't care about Object) code complexity and Flog#none is no method code complexity.

floggers.calls can also be useful. It returns hash like

{
"Flog#flog"=>{:assignment=>8.100000000000001, :expand_dirs_to_files=>1.1, :branch=>12.000000000000004, :each=>1.1, :===>1.4000000000000004, :read=>1.5000000000000004, :binread=>1.5000000000000004, :option=>5.100000000000001, :[]=>4.500000000000002, :warn=>12.200000000000005, :new=>1.4000000000000006, :process=>2.9000000000000012, :mass=>2.800000000000001, :inspect=>3.1000000000000014, :backtrace=>4.200000000000001, :lit_fixnum=>1.0500000000000003, :first=>3.800000000000001, :join=>3.4000000000000012, :raise=>1.7000000000000008, :class=>1.7000000000000006, :message=>1.9000000000000006, :strip=>1.7000000000000006}, 
"Flog::parse_options"=>{:assignment=>21.300000000000004, :branch=>25.400000000000002, :new=>1.3, :separator=>3.0000000000000004, :on=>19.500000000000004, :puts=>1.7000000000000004, :exit=>1.7000000000000004, :each=>3.1000000000000005, :<<=>1.7000000000000004, :plugins=>1.7000000000000002, :empty?=>1.5000000000000002, :methods=>1.9000000000000001, :grep=>1.7000000000000002, :-=>1.5000000000000002, :sort=>1.7000000000000002, :send=>4.800000000000001, :Array=>1.3, :parse!=>1.1},
"Flog::load_plugins"=>{:assignment=>9.8, :branch=>10.900000000000002, :find_files=>1.5, :reverse=>1.3, :each=>3.3000000000000003, :basename=>1.4000000000000001, :intern=>1.2000000000000002, :[]=>1.3000000000000003, :warn=>4.300000000000001, :load=>1.5000000000000004, :inspect=>1.6000000000000003, :message=>1.6000000000000003, :plugins=>1.3, :merge=>1.1, :constants=>1.5, :map=>1.3, :to_s=>1.4000000000000001, :reject=>1.1, :const_get=>1.3000000000000003, :====>1.3000000000000003}, 
"Flog#none"=>{:lit_fixnum=>11.799999999999994, :new=>1.1, :merge!=>3.3000000000000003, :attr_accessor=>1.1, :attr_reader=>2.2, :alias=>13.200000000000006}, 
"Flog#report"=>{:assignment=>2.300000000000001, :total=>3.1000000000000014, :%=>2.800000000000001, :puts=>2.4000000000000012, :average=>1.6000000000000005, :branch=>3.600000000000002, :option=>4.200000000000002, :[]=>3.600000000000002, :*=>1.3000000000000007, :output_details_grouped=>1.3000000000000007, :output_details=>1.3000000000000007, :reset=>1.1000000000000005}, 
"main#none"=>{:require=>5.5}
}

that shows why each method so complex.

@releu
Copy link
Member

releu commented Nov 5, 2012

rails bp

outputs YAML file looks like:

- !ruby/object:RailsBestPractices::Core::Error
  filename: ./app/helpers/comment_helper.rb
  line_number: '6'
  message: not use time_ago_in_words
  type: RailsBestPractices::Reviews::NotUseTimeAgoInWordsReview
  url: http://rails-bestpractices.com/posts/105-not-use-time_ago_in_words
  git_commit: 
  git_username: 
  hg_commit: 
  hg_username: 

also railsbp has a config file:

AddModelVirtualAttributeCheck: { }
AlwaysAddDbIndexCheck: { }
DryBundlerInCapistranoCheck: { }
#HashSyntaxCheck: { }
IsolateSeedDataCheck: { }
KeepFindersOnTheirOwnModelCheck: { }
LawOfDemeterCheck: { }
#LongLineCheck: { max_line_length: 80 }
MoveCodeIntoControllerCheck: { }
MoveCodeIntoHelperCheck: { array_count: 3 }
MoveCodeIntoModelCheck: { use_count: 2 }
MoveFinderToNamedScopeCheck: { }
MoveModelLogicIntoModelCheck: { use_count: 4 }
NeedlessDeepNestingCheck: { nested_count: 2 }
NotRescueExceptionCheck: { }
NotUseDefaultRouteCheck: {  }
NotUseTimeAgoInWordsCheck: {}
OveruseRouteCustomizationsCheck: { customize_count: 3 }
ProtectMassAssignmentCheck: {}
RemoveEmptyHelpersCheck: {}
#RemoveTabCheck: {}
RemoveTrailingWhitespaceCheck: { }
RemoveUnusedMethodsInControllersCheck: { except_methods: [] }
RemoveUnusedMethodsInHelpersCheck: { except_methods: [] }
RemoveUnusedMethodsInModelsCheck: { except_methods: [] }
ReplaceComplexCreationWithFactoryMethodCheck: { attribute_assignment_count: 2 }
ReplaceInstanceVariableWithLocalVariableCheck: { }
RestrictAutoGeneratedRoutesCheck: { }
SimplifyRenderInControllersCheck: {}
SimplifyRenderInViewsCheck: {}
#UseBeforeFilterCheck: { customize_count: 2 }
UseModelAssociationCheck: { }
UseMultipartAlternativeAsContentTypeOfEmailCheck: {}
UseObserverCheck: { }
#UseParenthesesInMethodDefCheck: {}
UseQueryAttributeCheck: { }
UseSayWithTimeInMigrationsCheck: { }
UseScopeAccessCheck: { }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants