diff --git a/README.md b/README.md index 68bddabd1..05ab3aa6b 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ ## Quickstart `reek` is a tool that examines Ruby classes, modules and methods and reports any -[Code Smells](https://github.com/troessner/reek/wiki/Code-Smells) it finds. +[Code Smells](docs/Code-Smells.md) it finds. Install it like this: ```Bash @@ -94,16 +94,16 @@ $stdin -- 3 warnings: ## Code smells `reek` currently includes checks for some aspects of -[Control Couple](https://github.com/troessner/reek/wiki/Control-Couple), -[Data Clump](https://github.com/troessner/reek/wiki/Data-Clump), -[Feature Envy](https://github.com/troessner/reek/wiki/Feature-Envy), -[Large Class](https://github.com/troessner/reek/wiki/Large-Class), -[Long Parameter List](https://github.com/troessner/reek/wiki/Long-Parameter-List), -[Simulated Polymorphism](https://github.com/troessner/reek/wiki/Simulated-Polymorphism), -[Too Many Statements](https://github.com/troessner/reek/wiki/Too-Many-Statements), -[Uncommunicative Name](https://github.com/troessner/reek/wiki/Uncommunicative-Name), -[Unused Parameters](https://github.com/troessner/reek/wiki/Unused-Parameters) -and more. See the [Code Smells](https://github.com/troessner/reek/wiki/Code-Smells) +[Control Couple](docs/Control-Couple.md), +[Data Clump](docs/Data-Clump.md), +[Feature Envy](docs/Feature-Envy.md), +[Large Class](docs/Large-Class.md), +[Long Parameter List](docs/Long-Parameter-List.md), +[Simulated Polymorphism](docs/Simulated-Polymorphism.md), +[Too Many Statements](docs/Too-Many-Statements.md), +[Uncommunicative Name](docs/Uncommunicative-Name.md), +[Unused Parameters](docs/Unused-Parameters.md) +and more. See the [Code Smells](docs/Code-Smells.md) for up to date details of exactly what `reek` will check in your code. ## Configuration @@ -116,7 +116,7 @@ For a basic overview, run reek --help ``` -For a summary of those CLI options see [Command-Line Options](https://github.com/troessner/reek/wiki/Command-Line-Options). +For a summary of those CLI options see [Command-Line Options](docs/Command-Line-Options.md). ### Configuration file @@ -145,12 +145,12 @@ of how many `*.reek` files you might have on your filesystem. #### Configuration options The first thing you probably want to check out are the -[Basic Smell Options](https://github.com/troessner/reek/wiki/Basic-Smell-Options) +[Basic Smell Options](docs/Basic-Smell-Options.md) which are supported by every smell type. Certain smell types offer a configuration that goes beyond that of the basic smell options, for instance -[Data Clump](https://github.com/troessner/reek/wiki/Data-Clump). -All options that go beyond the [Basic Smell Options](https://github.com/troessner/reek/wiki/Basic-Smell-Options) +[Data Clump](docs/Data-Clump.md). +All options that go beyond the [Basic Smell Options](docs/Basic-Smell-Options.md) should be documented in the corresponding smell type wiki page, but if you want to get a quick and full overview over all possible configurations you can always check out [the `config/default.reek` @@ -188,7 +188,7 @@ def smelly_method foo end ``` -This is further explained under [Smell Suppresion](https://github.com/troessner/reek/wiki/Smell-Suppression). +This is further explained under [Smell Suppresion](docs/Smell-Suppression.md). ## Integration @@ -201,8 +201,8 @@ reek [options] [dir_or_source_file]* there are quite a few other ways how to use `reek` in your projects: -* Use `reek`'s [Rake task](https://github.com/troessner/reek/wiki/Rake-Task) to automate detecting code smells -* Add `reek`'s custom matcher to your [RSpec examples](https://github.com/troessner/reek/wiki/RSpec-matchers) +* Use `reek`'s [Rake task](docs/Rake-Task.md) to automate detecting code smells +* Add `reek`'s custom matcher to your [RSpec examples](docs/RSpec-matchers.md) * Include `reek` using the [Developer API](docs/API.md) ## Developing `reek` / Contributing @@ -232,8 +232,8 @@ From then on please check out [the contributing guide](CONTRIBUTING.md). If you don't feel like getting your hands dirty with code there are still other ways you can help us: -* Work on the [wiki](https://github.com/troessner/reek/wiki) -* Open up an [issue](https://github.com/troessner/reek/issues) and report bugs or suggest other improvements +* Open up an [issue](https://github.com/troessner/reek/issues) and report bugs +* Suggest other improvements like additional smells for instance ## Output formats diff --git a/docs/API.md b/docs/API.md index 3867bda18..b79bb0552 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1,4 +1,6 @@ -# Using `reek` inside your Ruby application +# API + +## Using `reek` inside your Ruby application `reek` can be used inside another Ruby project. diff --git a/docs/Attribute.md b/docs/Attribute.md new file mode 100644 index 000000000..983b30417 --- /dev/null +++ b/docs/Attribute.md @@ -0,0 +1,43 @@ +# Attribute + +## Introduction + +A class that publishes a getter or setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state. + +## Example + +Given: + +```Ruby +class Klass + attr_accessor :dummy +end +``` + +`reek` would emit the following warning: + +``` +reek test.rb + +test.rb -- 1 warning: + [2]:Klass declares the attribute dummy (Attribute) +``` + +## Support in Reek + +Right now this smell is disabled by default since it is highly subjective. + +When this detector is enabled it raises a warning for every `attr`, `attr_reader`, `attr_writer` and `attr_accessor` -- including those that are private. + +## Configuration + +If you want to enable it you can do so by placing + +```yaml +Attribute: + enabled: true +``` + +in your reek configuration file. + +`Attribute` supports only the [Basic Smell Options](Basic-Smell-Options.md). diff --git a/docs/Basic-Smell-Options.md b/docs/Basic-Smell-Options.md new file mode 100644 index 000000000..e740317a9 --- /dev/null +++ b/docs/Basic-Smell-Options.md @@ -0,0 +1,44 @@ +# Basic Smell Options + +## Introduction + +Every smell detector in Reek offers at least the following configuration options: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `enabled` | Boolean | Determines whether the smell detector is active. Defaults to `true` | +| `exclude` | an array of strings or regular expressions | Ignores any context whose full description (see %c in [Command-Line Options](Command-Line-Options.md)) matches any element of this array. | + +The file `config/defaults.reek` (shipped with the Reek gem) lists any default exclusions for each smell. + +## Examples + +An easy one: + +To stop Reek reporting smells in any method called `write` you might create a configuration file containing this: + +```yaml +ControlCouple: + exclude: + - write +``` + +Or a little more sophisticated using a ruby regex like this: + +```yaml +ControlCouple: + exclude: + - !ruby/regexp /write/ +``` + +A more sophisticated one: + +```yaml +FeatureEnvy: + exclude: + - "MyModel#do_things" + - "MyHelper" + - "ApplicationController#respond" +``` + +This would not report FeatureEnvy for the instance method `MyModel#do_things`, the whole module `MyHelper` and the `respond` instance method of `ApplicationController` \ No newline at end of file diff --git a/docs/Boolean-Parameter.md b/docs/Boolean-Parameter.md new file mode 100644 index 000000000..368902981 --- /dev/null +++ b/docs/Boolean-Parameter.md @@ -0,0 +1,52 @@ +# Boolean Parameter + +## Introduction + +`Boolean Parameter` is a special case of [Control Couple](Control-Couple.md), where a method parameter is defaulted +to true or false. A _Boolean Parameter_ effectively permits a method's caller +to decide which execution path to take. This is a case of bad cohesion. You're creating a dependency between methods that is not really necessary, thus increasing coupling. + +## Example + +Given + +```Ruby +class Dummy + def hit_the_switch(switch = true) + if switch + puts 'Hitting the switch' + # do other things... + else + puts 'Not hitting the switch' + # do other things... + end + end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 3 warnings: + [1]:Dummy#hit_the_switch has boolean parameter 'switch' (BooleanParameter) + [2]:Dummy#hit_the_switch is controlled by argument switch (ControlParameter) +``` + +Note that both smells are reported, `Boolean Parameter` and `Control Parameter`. + +## Getting rid of the smell + +This is highly dependant on your exact architecture, but looking at the example above what you could do is: + +* Move everything in the `if` branch into a separate method +* Move everything in the `else` branch into a separate method +* Get rid of the `hit_the_switch` method alltogether +* Make the decision what method to call in the initial caller of `hit_the_switch` + +## Current support in Reek + +Reek can only detect a Boolean parameter when it has a default initializer like in the example above. + +## Configuration + +`Boolean Parameter` supports the [Basic Smell Options](Basic-Smell-Options.md). diff --git a/docs/Class-Variable.md b/docs/Class-Variable.md new file mode 100644 index 000000000..b57dbd49d --- /dev/null +++ b/docs/Class-Variable.md @@ -0,0 +1,40 @@ +# Class Variable + +## Introduction + +Class variables form part of the global runtime state, and as such make it easy for one part of the system to accidentally or inadvertently depend on another part of the system. So the system becomes more prone to problems where changing something over here breaks something over there. In particular, class variables can make it hard to set up tests (because the context of the test includes all global state). + +For a detailed explanation, check out [this article](http://4thmouse.com/index.php/2011/03/20/why-class-variables-in-ruby-are-a-bad-idea/) + +## Example + +Given + +```Ruby +class Dummy + @@class_variable = :whatever +end +``` + +`reek` would emit the following warning: + +``` +reek test.rb + +test.rb -- 1 warning: + [2]:Dummy declares the class variable @@class_variable (ClassVariable) +``` + +## Getting rid of the smell + +You can use class-instance variable to mitigate the problem (as also suggested in the linked article above): + +```Ruby +class Dummy + @class_variable = :whatever +end +``` + +## Configuration + +`Class Variable` supports the [Basic Smell Options](Basic-Smell-Options.md). diff --git a/docs/Code-Smells.md b/docs/Code-Smells.md new file mode 100644 index 000000000..f505f26d5 --- /dev/null +++ b/docs/Code-Smells.md @@ -0,0 +1,34 @@ +# Code Smells + +Smells are indicators of where your code might be hard to read, maintain or evolve, rather than things that are specifically _wrong_. Naturally this means that Reek is looking towards your code's future (and that can make its reports seem somewhat subjective, of course). + +Reek currently includes checks for the following smells: + +* [Attribute](Attribute.md) (disabled by default) +* [Class Variable](Class-Variable.md) +* [Control Couple](Control-Couple.md), including + * [Boolean Parameter](Boolean-Parameter.md) + * [Control Parameter](Control-Parameter.md) +* [Data Clump](Data-Clump.md) +* [Duplicate Method Call](Duplicate-Method-Call.md) +* [Irresponsible Module](Irresponsible-Module.md) +* [Large Class](Large-Class.md), including + * [Too Many Instance Variables](Too-Many-Instance-Variables.md) + * [Too Many Methods](Too-Many-Methods.md) +* [Long Parameter List](Long-Parameter-List.md), and its special case [Long Yield List](Long-Yield-List.md) +* Low Cohesion, including + * [Feature Envy](Feature-Envy.md) + * [Utility Function](Utility-Function.md) +* [Module Initialize](Module-Initialize.md) +* [Nested Iterators](Nested-Iterators.md) +* [Prima-Donna-Method](Prima-Donna-Method.md) +* [Simulated Polymorphism](Simulated-Polymorphism.md), including + * [Nil Check](Nil-Check.md) + * [Repeated Conditional](Repeated-Conditional.md) +* [Too Many Statements](Too-Many-Statements.md) +* [Uncommunicative Name](Uncommunicative-Name.md), including + * [Uncommunicative Method Name](Uncommunicative-Method-Name.md) + * [Uncommunicative Module Name](Uncommunicative-Module-Name.md) + * [Uncommunicative Parameter Name](Uncommunicative-Parameter-Name.md) + * [Uncommunicative Variable Name](Uncommunicative-Variable-Name.md) +* [Unused Parameters](Unused-Parameters.md) \ No newline at end of file diff --git a/docs/Command-Line-Options.md b/docs/Command-Line-Options.md new file mode 100644 index 000000000..9f2505955 --- /dev/null +++ b/docs/Command-Line-Options.md @@ -0,0 +1,84 @@ +# Command Line Options + +## Introduction + +reek follows standard Unix convention for passing arguments. + +Check out + +```Bash +reek -h +``` + +for details. + +## Telling Reek Which Code to Check + +Probably the most standard use case would be to check all ruby files in the lib directory: + +```Bash +reek lib/*.rb +``` + +In general, if any command-line argument is a directory, Reek searches that directory and all sub-directories for Ruby source files. Thus + +```Bash +reek lib +``` + +would be equivalent to + +```Bash +reek lib/**/*.rb +``` + +Occasionally you may want to quickly check a code snippet without going to the trouble of creating a file to hold it. You can pass the snippet directly to Reek's standard input: + +```Bash +echo "def x() true end" | reek +``` + +## Output options + +### Output smell's line number + +By passing in a "-n" flag to the _reek_ command, the output will suppress the line numbers: + +```Bash +$ reek -n mess.rb +``` + +``` +mess.rb -- 2 warnings: + x doesn't depend on instance state (UtilityFunction) + x has the name 'x' (UncommunicativeMethodName) +``` + +Otherwise line numbers will be shown as default at the beginning of each warning in square brackets: + +```Bash +$ reek mess.rb +``` + +``` +mess.rb -- 2 warnings: + [2]:x doesn't depend on instance state (UtilityFunction) + [2]:x has the name 'x' (UncommunicativeMethodName) +``` + +### Enable the ultra-verbose mode + +_reek_ has a ultra-verbose mode which you might find helpful as a beginner. "ultra-verbose" just means that behind each warning a helpful link will be displayed which leads directly to the corresponding _reek_ wiki page. +This mode can be enabled via the "-U" or "--ultra-verbose" flag. + +So for instance, if your test file would smell of _ClassVariable_, this is what the _reek_ output would look like: + +```Bash +reek -U test.rb +``` +``` +test.rb -- 1 warning: + [2]:Dummy declares the class variable @@class_variable (ClassVariable) [https://github.com/troessner/reek/wiki/Class-Variable] +``` + +Note the link at the end. \ No newline at end of file diff --git a/docs/Configuration-Files.md b/docs/Configuration-Files.md new file mode 100644 index 000000000..a14d611c7 --- /dev/null +++ b/docs/Configuration-Files.md @@ -0,0 +1,38 @@ +# Configuration Files + +## Configuration loading + +Configuring `reek` via configuration file is by far the most powerful way. + +There are 3 ways of passing `reek` a configuration file: + +1. Using the cli "-c" switch (see [Command Line Options](Command-Line-Options.md)) +2. Having a file ending with .reek either in your current working directory or in a parent directory (more on that later) +3. Having a file ending with .reek in your HOME directory + +The order in which `reek` tries to find such a configuration file is exactly like above: First `reek` checks if we have given it a configuration file explicitly via CLI. Then it checks the current working directory for a file and if it can't find one, it traverses up the directories until it hits the root directory. And lastly, it checks your HOME directory. + +As soon as `reek` detects a configuration file it stops searching immediately, meaning that from `reek`'s point of view there exists one configuration file and one configuration only regardless of how many ".reek" files you might have on your filesystem. + +## Configuration options + +The first thing you probably want to check out are the [Basic Smell Options](Basic-Smell-Options.md) which are supported by every smell type. +Certain smell types offer a configuration that goes beyond that of the basic smell options - for instance [Data Clump](Data-Clump.md). +All options that go beyond the [Basic Smell Options](Basic-Smell-Options.md) should be documented in the corresponding smell type wiki page but if you want to get a quick and full overview over all possible configurations you can always check out [the default.reek file in this repository](https://github.com/troessner/reek/blob/master/config/defaults.reek). + +Here's an excerpt of a `reek` configuration file from a commercial project: + +```yaml +--- +IrresponsibleModule: + enabled: false +NestedIterators: + exclude: + - "ActiveModelErrorAdder#self.run" # should be refactored + - "BookingRequests::Transfer#remote_validation" + - "BookingRequestsController#vehicle_options" # respond_to block + - "Content::Base#self.expose_fields" # unavoidable due to metaprogramming +DataClump: + max_copies: 3 + min_clump_size: 3 +``` diff --git a/docs/Control-Couple.md b/docs/Control-Couple.md new file mode 100644 index 000000000..98737a15f --- /dev/null +++ b/docs/Control-Couple.md @@ -0,0 +1,22 @@ +# Control Couple + +## Introduction + +Control coupling occurs when a method or block checks the value of a parameter in order to decide which execution path to take. The offending parameter is often called a `Control Couple`. + +Control Coupling is a kind of duplication, because the calling method already knows which path should be taken. + +Control Coupling reduces the code's flexibility by creating a dependency between the caller and callee: any change to the possible values of the controlling parameter must be reflected on both sides of the call. A `Control Couple` also reveals a loss of simplicity: the called method probably has more than one responsibility, because it includes at least two different code paths. + +You can find a good write-up regarding this problem [here](http://solnic.eu/2012/04/11/get-rid-of-that-code-smell-control-couple.html). + +## Current Support in reek + +`reek` warns about control coupling when: + +* [Control-Parameter](Control-Parameter.md) - a method parameter or block parameter is the tested value in a conditional statement (as in the example below); or +* [Boolean-Parameter](Boolean-Parameter.md) - a method parameter is defaulted to `true` or `false`. + +## Configuration + +Control Couple supports the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/Control-Parameter.md b/docs/Control-Parameter.md new file mode 100644 index 000000000..74a0ebe6b --- /dev/null +++ b/docs/Control-Parameter.md @@ -0,0 +1,29 @@ +# Control Parameter + +## Introduction + +`Control Parameter` is a special case of [Control Couple](Control-Couple.md) + +## Example + +A simple example would be the "quoted" parameter in the following method: + +```Ruby +def write(quoted) + if quoted + write_quoted @value + else + write_unquoted @value + end +end +``` + +Fixing those problems is out of the scope of this document but an easy solution could be to remove the "write" method alltogether and to move the calls to "write_quoted" / "write_unquoted" in the initial caller of "write". + +## Current Support in reek + +`reek` warns about control coupling when a method parameter or block parameter is the tested value in a conditional statement. + +## Configuration + +Control Couple supports the [Basic Smell Options](Basic-Smell-Options.md). diff --git a/docs/Data-Clump.md b/docs/Data-Clump.md new file mode 100644 index 000000000..c9af81e6c --- /dev/null +++ b/docs/Data-Clump.md @@ -0,0 +1,44 @@ +# Data Clump + +## Introduction + +In general, a `Data Clump` occurs when the same two or three items frequently appear together in classes and parameter lists, or when a group of instance variable names start or end with similar substrings. + +The recurrence of the items often means there is duplicate code spread around to handle them. There may be an abstraction missing from the code, making the system harder to understand. + +## Example + +Given + +```Ruby +class Dummy + def x(y1,y2); end + def y(y1,y2); end + def z(y1,y2); end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 1 warning: + [2, 3, 4]:Dummy takes parameters [y1, y2] to 3 methods (DataClump) +``` + +A possible way to fix this problem (quoting from [Martin Fowler](http://martinfowler.com/bliki/DataClump.html)): + +>> The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects. + +## Current Support in Reek + +`reek` looks for a group of two or more parameters with the same names that are expected by three or more methods of a class. + +## Configuration + +Reek's Data Clump detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_copies | integer | The maximum number of methods that are permitted to take the same group of parameters. Defaults to 2 | +| min_clump_size | integer | The smallest number of parameters that can be reported as a clump. Defaults to 2 | + diff --git a/docs/Duplicate-Method-Call.md b/docs/Duplicate-Method-Call.md new file mode 100644 index 000000000..db69964b2 --- /dev/null +++ b/docs/Duplicate-Method-Call.md @@ -0,0 +1,49 @@ +# Duplicate Method Call + +## Introduction + +Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level. +`reek` implements a check for _Duplicate Method Call_. + +## Example + +Here's a very much simplified and contrived example. The following method will report a warning: + +```Ruby +def double_thing() + @other.thing + @other.thing +end +``` + +One quick approach to silence Reek would be to refactor the code thus: + +```Ruby +def double_thing() + thing = @other.thing + thing + thing +end +``` + +A slightly different approach would be to replace all calls of `double_thing` by calls to `@other.double_thing`: + +```Ruby +class Other + def double_thing() + thing + thing + end +end +``` + +The approach you take will depend on balancing other factors in your code. + +## Current support in reek + +`reek`'s Duplicate Method Call detector checks for repeated identical method calls within any one method definition. This is intended to complement the checks performed by tools such as [Flay](http://ruby.sadi.st/Flay.html) and [Simian](http://www.redhillconsulting.com.au/products/simian/). + +## Configuration + +Reek's Duplication detector currently offers the [Basic Smell Options](Basic-Smell-Options.md), plus: + +Option | Value | Effect +-------|-------|------- +`max_calls` | integer | The maximum number of duplicate calls allowed within a method. Defaults to 1. \ No newline at end of file diff --git a/docs/Feature-Envy.md b/docs/Feature-Envy.md new file mode 100644 index 000000000..b3228d59f --- /dev/null +++ b/docs/Feature-Envy.md @@ -0,0 +1,29 @@ +# Feature Envy + +## Introduction + +Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object. + +A simple example would be the following method, which "belongs" on the Item class and not on the Cart class: + +```Ruby +class Cart + def price + @item.price + @item.tax + end +end +``` + +`Feature Envy` reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class. + +`Feature Envy` also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class. + +`Feature Envy` often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there. + +## Current Support in reek + +`Feature Envy` reports any method that refers to self less often than it refers to (ie. send messages to) some other object. + +## Configuration + +Feature Envy supports the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/How-reek-works-internally.md b/docs/How-reek-works-internally.md new file mode 100644 index 000000000..7f7509682 --- /dev/null +++ b/docs/How-reek-works-internally.md @@ -0,0 +1,44 @@ +# How reek works internally + +**Using reek via bin/reek:** + +``` + [bin/reek] + | + | + | + Application (cli/application.rb) + + Options (cli/options) + | + | + | + ReekCommand (cli/reek_command) + with Reporter (cli/report/report) + / | \ + / | \ + / | \ + Source Source Source (source/source_code) + | | | + | | | + | | | + Examiner Examiner Examiner (examiner) + | + | + | + Examiner sets up a: + - SourceRepository (source/source_repository) + - a WarningCollector (core/warning_collector) + + The Examiner then goes through each source: + - Initializing a SmellRepository (core/smell_repository) + - getting the AST from the source + - applying a TreeWalker(core/tree_walker) to process this syntax tree given the SmellRepository + - finally have that SmellRepository reporting back on the WarningCollector mentioned above + | + | + | + In the last step, the reporter from the ReekCommand: + - gathers all the warnings from the collectors of all Examiners (as you can see here https://github.com/troessner/reek/blob/master/lib/reek/cli/report/report.rb#L30) + - outputs them with whatever output format we have chose via the cli options +``` + diff --git a/docs/Irresponsible-Module.md b/docs/Irresponsible-Module.md new file mode 100644 index 000000000..d4cc01448 --- /dev/null +++ b/docs/Irresponsible-Module.md @@ -0,0 +1,39 @@ +# Irresponsible Module + +## Introduction + +Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities. + +## Example + +Given + +```Ruby +class Dummy + # Do things... +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 1 warning: + [1]:Dummy has no descriptive comment (IrresponsibleModule) +``` + +Fixing this is simple - just an explaining comment: + +```Ruby +# The Dummy class is responsible for ... +class Dummy + # Do things... +end +``` + +## Current Support in reek + +`Irresponsible Module` currently checks classes, but not modules. + +## Configuration + +`Irresponsible Module` supports only the [Basic Smell Options](Basic-Smell-Options.md). diff --git a/docs/Large-Class.md b/docs/Large-Class.md new file mode 100644 index 000000000..9af2121e4 --- /dev/null +++ b/docs/Large-Class.md @@ -0,0 +1,20 @@ +# Large Class + +## Introduction + +A `Large Class` is a class or module that has a large number of instance variables, methods or lines of code in any one piece of its specification. (That is, this smell relates to pieces of the class's specification, not to the size of the corresponding instance of `Class`.) + +## Current Support in Reek + +`Large Class` reports classes having more than a configurable number of methods or instance variables. The method count includes public, protected and private methods, and excludes methods inherited from superclasses or included modules. + +## Configuration + +`reek`'s Large Class detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_methods | integer | The maximum number of methods allowed in a class before a warning is issued. Defaults to 25. | +| max_instance_variables | integer | The maximum number of instance variables allowed in a class before a warning is issued. Defaults to 9. | + +The `Large Class` detector is enabled whenever `reek` is asked to check an instance of `Class` or `Module`. \ No newline at end of file diff --git a/docs/Long-Parameter-List.md b/docs/Long-Parameter-List.md new file mode 100644 index 000000000..7f990eb4b --- /dev/null +++ b/docs/Long-Parameter-List.md @@ -0,0 +1,38 @@ +# Long Parameter List + +## Introduction + +A `Long Parameter List` occurs when a method has a lot of parameters. + +## Example + +Given + +```Ruby +class Dummy + def long_list(foo,bar,baz,fling,flung) + puts foo,bar,baz,fling,flung + end +end +``` + +`reek` would report the following warning: + +``` +test.rb -- 1 warning: + [2]:Dummy#long_list has 5 parameters (LongParameterList) +``` + +A common solution to this problem would be the introduction of parameter objects. + +## Current Support in Reek + +`Long Parameter List` reports any method or block with more than 3 parameters. + +## Configuration + +Reek's Long Parameter List detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_params | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. | diff --git a/docs/Long-Yield-List.md b/docs/Long-Yield-List.md new file mode 100644 index 000000000..569d9e70f --- /dev/null +++ b/docs/Long-Yield-List.md @@ -0,0 +1,36 @@ +# Long Yield List + +## Introduction + +A _Long Yield List_ occurs when a method yields a lot of arguments to the block it gets passed. + +## Example + +```Ruby +class Dummy + def yields_a_lot(foo,bar,baz,fling,flung) + yield foo,bar,baz,fling,flung + end +end +``` + +`reek` would report the following warning: + +``` +test.rb -- 1 warning: + [4]:Dummy#yields_a_lot yields 5 parameters (LongYieldList) +``` + +A common solution to this problem would be the introduction of parameter objects. + +## Current Support in Reek + +Currently Long Parameter List reports any method or block with more than 3 parameters. + +## Configuration + +Reek's Long Parameter List detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_params | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. | \ No newline at end of file diff --git a/docs/Module-Initialize.md b/docs/Module-Initialize.md new file mode 100644 index 000000000..6c2913151 --- /dev/null +++ b/docs/Module-Initialize.md @@ -0,0 +1,62 @@ +# Module Initialize + +## Introduction + +A module is usually a mixin, so when an `#initialize` method is present it is +hard to tell initialization order and parameters so having `#initialize` +in a module is usually a bad idea. + +## Example + +The `Foo` module below contains a method `initialize`. Although class `B` inherits from `A`, the inclusion of `Foo` stops `A#initialize` from being called. + +```Ruby +class A + def initialize(a) + @a = a + end +end + +module Foo + def initialize(foo) + @foo = foo + end +end + +class B < A + include Foo + + def initialize(b) + super('bar') + @b = b + end +end +``` + +A simple solution is to rename `Foo#initialize` and call that method by name: + +```Ruby +module Foo + def setup_foo_module(foo) + @foo = foo + end +end + +class B < A + include Foo + + def initialize(b) + super 'bar' + setup_foo_module('foo') + @b = b + end +end +``` + +## Current Support in reek + +`reek` warns about module initialize when an instance method named `initialize` is present in a module. + +## Configuration + +Module Initialize supports the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/Nested-Iterators.md b/docs/Nested-Iterators.md new file mode 100644 index 000000000..020f6367d --- /dev/null +++ b/docs/Nested-Iterators.md @@ -0,0 +1,38 @@ +# Nested Iterators + +## Introduction + +A `Nested Iterator` occurs when a block contains another block. + +## Example + +Given + +```Ruby +class Duck + class << self + def duck_names + %i!tick trick track!.each do |surname| + %i!duck!.each do |last_name| + puts "full name is #{surname} #{last_name}" + end + end + end + end +end +``` + +`reek` would report the following warning: + +``` +test.rb -- 1 warning: + [5]:Duck#duck_names contains iterators nested 2 deep (NestedIterators) +``` + +## Current Support in Reek + +Nested Iterators reports failing methods only once. + +## Configuration + +`Nested Iterators` offers the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/Nil-Check.md b/docs/Nil-Check.md new file mode 100644 index 000000000..ca12dce2c --- /dev/null +++ b/docs/Nil-Check.md @@ -0,0 +1,39 @@ +# Nil Check + +## Introduction + +A `NilCheck` is a type check. Failures of `NilCheck` violate the ["tell, don't ask"](http://robots.thoughtbot.com/tell-dont-ask) principle. +Additionally to that, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should. + +## Example + +Given + +```Ruby +class Klass + def nil_checker(argument) + if argument.nil? + puts "argument isn't nil!" + end + end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 1 warning: + [3]:Klass#nil_checker performs a nil-check. (NilCheck) +``` + +## Current Support in Reek + +`NilCheck` reports use of + +* .nil? method +* == and === operators when checking vs. nil +* case statements that use syntax like when nil + +## Configuration + +`Nil Check` offers the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/Non-ASCII-source-files.md b/docs/Non-ASCII-source-files.md new file mode 100644 index 000000000..aca149362 --- /dev/null +++ b/docs/Non-ASCII-source-files.md @@ -0,0 +1,15 @@ +# Non ASCII source files + + +`reek` doesn't offer a direct way to specify that your source file is encoded as UTF-8 etc, but there is a workaround if you need it: + +`reek` includes the class [Reek::RakeTask](http://reek.rubyforge.org/rdoc/classes/Reek/RakeTask.html), which makes it easy to run `reek` from a rakefile. And this task has an attribute `ruby_opts`, which is an array of strings to be passed as arguments directly to the Ruby interpreter. So you can use `reek` to check for smells in non-ASCII files by adding something like this to your rakefile: + +```Ruby +require 'reek/rake_task' + +Reek::RakeTask.new('utf8_file') do |t| + t.source_files = "my_utf8_file.rb" + t.ruby_opts = ["-Ku"] +end +``` diff --git a/docs/Prima-Donna-Method.md b/docs/Prima-Donna-Method.md new file mode 100644 index 000000000..a124cfb4e --- /dev/null +++ b/docs/Prima-Donna-Method.md @@ -0,0 +1,53 @@ +# Prima Donna Method + +## Introduction + +A candidate method for the `Prima Donna Method` smell are methods whose names end with an exclamation mark. + +An exclamation mark in method names means (the explanation below is taken from [here](http://dablog.rubypal.com/2007/8/15/bang-methods-or-danger-will-rubyist) ): + +>> +The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. +So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth. + +Such a method is called `Prima Donna Method` if and only if her non-bang version does not exist and this method is reported as a smell. + +## Example + +Given + +```Ruby +class C + def foo; end + def foo!; end + def bar!; end +end +``` + +`reek` would report `bar!` as `prima donna method` smell but not `foo!`. + +`reek` reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this: + + +```Ruby +class Parent + def foo; end +end + +module Dangerous + def foo!; end +end + +class Son < Parent + include Dangerous +end + +class Daughter < Parent +end +``` + +In this example, `reek` would not report the `prima donna method` smell for the method `foo` of the `Dangerous` module. + +## Configuration + +`Prima Donna Method` offers the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/RSpec-matchers.md b/docs/RSpec-matchers.md new file mode 100644 index 000000000..6a44a7d83 --- /dev/null +++ b/docs/RSpec-matchers.md @@ -0,0 +1,133 @@ +# RSpec matchers + +## Introduction + +`reek` offers matchers for RSpec you can easily include into your project. + +There are 3 matchers available: + +- `reek` +- `reek_of` +- `reek_only_of` + +## Quickstart + +Let's install the dependencies: + +``` +gem install reek +gem install rspec +``` + +And then use it like that in your spec file: + +```Ruby +require 'reek' +require 'reek/spec' +require 'rspec' + +RSpec.describe 'Reek Integration' do + it 'works with reek' do + smelly_class = 'class C; def m; end; end' + expect(smelly_class).not_to reek + end +end +``` + +Running this via + +``` +rspec reek-integration-spec.rb +``` + +would give you: + +``` +Failures: + + 1) Reek Integration works with reek + Failure/Error: expect(smelly_class).not_to reek + Expected no smells, but got: + C has no descriptive comment (IrresponsibleModule) + C has the name 'C' (UncommunicativeModuleName) + C#m has the name 'm' (UncommunicativeMethodName) + # ./reek-integration-spec.rb:8:in `block (2 levels) in ' + +Finished in 0.00284 seconds (files took 0.28815 seconds to load) +1 example, 1 failure + +Failed examples: + +rspec ./reek-integration-spec.rb:6 # Reek Integration works with reek +``` + +## The matchers explained + +### `reek` + +A very generic matcher that basically just tells you if something reeks, but not after what exactly. +See the "Quickstart" example from above. + +### `reek_of` + +Checks the target source code for instances of "smell category" +and returns true only if it can find one of them that matches. + +Remember that this includes our "smell types" as well. So it could be the +"smell type" UtilityFunction, which is represented as a concrete class +in reek but it could also be "Duplication" which is a "smell categgory". + +In theory you could pass many different types of input here: + - :UtilityFunction + - "UtilityFunction" + - UtilityFunction (this works in our specs because we tend to do "include Reek:Smells") + - Reek::Smells::UtilityFunction (the right way if you really want to pass a class) + - "Duplication" or :Duplication which is an abstract "smell category" + +It is recommended to pass this as a symbol like :UtilityFunction. However we don't +enforce this. + +Additionally you can be more specific and pass in "smell_details" you +want to check for as well e.g. "name" or "count" (see the examples below). +The parameters you can check for are depending on the smell you are checking for. +For instance "count" doesn't make sense everywhere whereas "name" does in most cases. +If you pass in a parameter that doesn't exist (e.g. you make a typo like "namme") reek will +raise an ArgumentError to give you a hint that you passed something that doesn't make +much sense. + +So in a nutshell `reek_of` takes the following two arguments: + +- smell_category - The "smell category" or "smell_type" we check for. +- smells_details - A hash containing "smell warning" parameters + +**Examples** + + Without smell_details: + +```Ruby + reek_of(:FeatureEnvy) + reek_of(Reek::Smells::UtilityFunction) +``` + +With smell_details: + +```Ruby + reek_of(UncommunicativeParameterName, name: 'x2') + reek_of(DataClump, count: 3) +``` + +**Examples from a real spec** + +```Ruby + expect(src).to reek_of(Reek::Smells::DuplicateMethodCall, name: '@other.thing') +``` + +### reek_only_of + +See the documentaton for `reek_of`. + +**Notable differences to reek_of:** + +1.) `reek_of` doesn't mind if there are other smells of a different category. "reek_only_of" will fail in that case. + +2.) `reek_only_of` doesn't support the additional smell_details hash. \ No newline at end of file diff --git a/docs/Rake-Task.md b/docs/Rake-Task.md new file mode 100644 index 000000000..707e03bd2 --- /dev/null +++ b/docs/Rake-Task.md @@ -0,0 +1,58 @@ +# Rake Task + +## Introduction + +`reek` provides a Rake task that runs `reek` on a set of source files. In its most simple form you just include something like that in your Rakefile: + +```Ruby +require 'reek/rake/task' + +Reek::Rake::Task.new do |t| + t.fail_on_error = false +end +``` + +In its most simple form, that's it. + +When you now run: + +```Bash +rake -T +``` + +you should see + +```Bash +rake reek # Check for code smells +``` + +## Configuration via task + +An more sophisticated rake task that would make use of all available configuration options could look like this: + +```Ruby +Reek::Rake::Task.new do |t| + t.name = 'custom_rake' # Whatever name you want. Defaults to "reek". + t.config_file = 'config/config.reek' # Defaults to nothing. + t.source_files = 'vendor/**/*.rb' # Glob pattern to match source files. Defaults to lib/**/*.rb + t.reek_opts = '-U' # Defaults to ''. You can pass all the options here in that are shown by "reek -h" + t.fail_on_error = false # Defaults to true + t.verbose = true # Defaults to false +end +``` + +## Configuration via environment variables + +You can overwrite the following attributes by environment variables: + +- "reek_opts" by using REEK_OPTS +- "config_file" by using REEK_CFG +- "source_files" by using REEK_SRC + +An example rake call using environment variables could look like this: + +```Bash +REEK_CFG="config/custom.reek" REEK_OPTS="-s" rake reek +``` + +See also: [Reek-Driven-Development](Reek-Driven-Development.md) \ No newline at end of file diff --git a/docs/Reek-Driven-Development.md b/docs/Reek-Driven-Development.md new file mode 100644 index 000000000..a55bee6ba --- /dev/null +++ b/docs/Reek-Driven-Development.md @@ -0,0 +1,45 @@ +# Reek Driven Development + +## rake + +One way to drive quality into your code from the very beginning of a project is to run `reek` as a part of your testing process. For example, you could do that by adding a [Rake Task](Rake-Task.md) to your rakefile, which will make it easy to run `reek` on all your source files whenever you need to. + +```Ruby +require 'reek/rake/task' + +Reek::Rake::Task.new do |t| + t.fail_on_error = true + t.verbose = false + t.source_files = 'lib/**/*.rb' +end +``` + +Now the command `reek` will run `reek` on your source code (and in this case, it fails if it finds any smells). For more detailed information about `reek`'s integration with Rake, see [Rake Task](Rake-Task.md) in this wiki. + +## reek/spec + +But there's another way; a much more effective "Reek-driven" approach: add `reek` expectations directly into your Rspec specs. Here's an example taken directly from `reek`'s own source code: + +```Ruby +it 'contains no code smells' do + Dir['lib/**/*.rb'].should_not reek +end +``` + +By requiring "reek/spec":http://reek.rubyforge.org/rdoc/classes/Reek/Spec.html you gain access to the `reek` matcher, which returns true if and only if `reek` finds smells in your code. And if the test fails, the matcher produces an error message that includes details of all the smells it found. + +Note: if you're on ruby 1.9 and RSpec2 you should include Reek::Spec in the configuration block like so, + +```Ruby +RSpec.configure do |c| + c.include(Reek::Spec) +end +``` + +## assert + +If you're not yet into BDD with Rspec, you can still gain the benefits of Reek-driven development using assertions: + +```Ruby +assert !Dir['lib/**/*.rb'].to_source.smelly? +``` diff --git a/docs/Repeated-Conditional.md b/docs/Repeated-Conditional.md new file mode 100644 index 000000000..314254e42 --- /dev/null +++ b/docs/Repeated-Conditional.md @@ -0,0 +1,44 @@ +# Repeated Conditional + +## Introduction + +`Repeated Conditional` is a special case of [Simulated Polymorphism](Simulated-Polymorphism.md). Basically it means you are checking the same value throughout a single class and take decisions based on this. + +## Example + +Given + +```Ruby +class RepeatedConditionals + attr_accessor :switch + + def repeat_1 + puts "Repeat 1!" if switch + end + + def repeat_2 + puts "Repeat 2!" if switch + end + + def repeat_3 + puts "Repeat 3!" if switch + end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 4 warnings: + [5, 9, 13]:RepeatedConditionals tests switch at least 3 times (RepeatedConditional) +``` + +If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction. + +## Configuration + +`reek`'s `Repeated Conditional` detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `max_ifs` | integer | The maximum number of identical conditional tests permitted before Reek raises a warning. Defaults to 2. | diff --git a/docs/Simulated-Polymorphism.md b/docs/Simulated-Polymorphism.md new file mode 100644 index 000000000..bd12e5fd3 --- /dev/null +++ b/docs/Simulated-Polymorphism.md @@ -0,0 +1,16 @@ +# Simulated Polymorphism + +## Introduction + +Simulated Polymorphism occurs when + +* code uses a case statement (especially on a type field); +* or code has several if statements in a row (especially if they're comparing against the same value); +* or code uses instance_of?, kind_of?, is_a?, or === to decide what type it's working with; +* or multiple conditionals in different places test the same value. + +Conditional code is hard to read and understand, because the reader must hold more state in his head. When the same value is tested in multiple places throughout an application, any change to the set of possible values will require many methods and classes to change. Tests for the type of an object may indicate that the abstraction represented by that type is not completely defined (or understood). + +## Current Support in reek + +`reek` checks for [Repeated Conditional](Repeated-Conditional.md) and for [Nil Check](Nil-Check.md). diff --git a/docs/Smell-Suppression.md b/docs/Smell-Suppression.md new file mode 100644 index 000000000..893f23c90 --- /dev/null +++ b/docs/Smell-Suppression.md @@ -0,0 +1,32 @@ +## Introduction + +In some cases, it might be necessary to suppress one or more of `reek`'s smell warnings for a particular method or class. + +Possible reasons for this could be: + +* The code is outside of your control and you can't fix it +* `reek` is not the police. You might have legit reasons why your source code is good as it is. + +## How to disable smell detection + +First and foremost, there are the [Basic Smell Options](Basic-Smell-Options.md) you can use. + +Besides from that, you can use special comments, like so: + +```ruby +# This method smells of :reek:NestedIterators +def smelly_method foo + foo.each {|bar| bar.each {|baz| baz.qux}} +end +``` + +The method `smelly_method` will not be reported. The general pattern is to put the string ':reek:', followed by the smell class, in a comment before the method or class. + +It is also possible to specify options for a particular smell detector, like so: + +```ruby +# :reek:LongParameterList: { max_params: 4 } +def many_parameters_it_has foo, bar, baz, qux + # ... +end +``` diff --git a/docs/Too-Many-Instance-Variables.md b/docs/Too-Many-Instance-Variables.md new file mode 100644 index 000000000..f8bd72e77 --- /dev/null +++ b/docs/Too-Many-Instance-Variables.md @@ -0,0 +1,43 @@ +## Introduction + +`Too Many Instance Variables` is a special case of `LargeClass`. + +## Example + +Given this configuration + +```yaml +TooManyInstanceVariables: + max_instance_variables: 3 +``` + +and this code: + +```Ruby +class TooManyInstanceVariables + def initialize + @arg_1 = :dummy + @arg_2 = :dummy + @arg_3 = :dummy + @arg_4 = :dummy + end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 5 warnings: + [1]:TooManyInstanceVariables has at least 4 instance variables (TooManyInstanceVariables) +``` +## Current Support in `reek` + +`reek` only counts the instance variables you use explicitly like in the example above. Class macros like `attr_accessor` are disregarded. + +## Configuration + +`reek`'s `Too Many Instance Variables` detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_instance_variables | integer | The maximum number of instance variables that are permitted. Defaults to 9 | diff --git a/docs/Too-Many-Methods.md b/docs/Too-Many-Methods.md new file mode 100644 index 000000000..b4c46483d --- /dev/null +++ b/docs/Too-Many-Methods.md @@ -0,0 +1,55 @@ +## Introduction + +`Too Many Methods` is a special case of `LargeClass`. + +## Example + +Given this configuration + +```yaml +TooManyMethods: + max_methods: 3 +``` + +and this code: + +```Ruby +class TooManyMethods + def one; end + def two; end + def three; end + def four; end +end +``` + +`reek` would emit the following warning: + +``` +test.rb -- 1 warning: + [1]:TooManyMethods has at least 4 methods (TooManyMethods) +``` +## Current Support in `reek` + +`reek` counts all the methods it can find in a `class` - instance *and* class methods. So given `max_methods` from above is 4, this: + +```Ruby +class TooManyMethods + class << self + def one; end + def two; end + end + + def three; end + def four; end +end +``` + +would cause reek to emit the same warning as in the example above. + +## Configuration + +`reek`'s `Too Many Methods` detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| max_methods | integer | The maximum number of methods that are permitted. Defaults to 25 | diff --git a/docs/Too-Many-Statements.md b/docs/Too-Many-Statements.md new file mode 100644 index 000000000..65654f393 --- /dev/null +++ b/docs/Too-Many-Statements.md @@ -0,0 +1,50 @@ +# Too Many Statements + +## Introduction + +A method with `Too Many Statements` is any method that has a large number of lines. + +## Current Support in Reek + +`Too Many Statements` warns about any method that has more than 5 statements. `reek`'s smell detector for `Too Many Statements` counts +1 for every simple statement in a method and +1 for every statement within a control structure (`if`, `else`, `case`, `when`, `for`, `while`, `until`, `begin`, `rescue`) but it doesn't count the control structure itself. + +So the following method would score +6 in Reek's statement-counting algorithm: + +```Ruby +def parse(arg, argv, &error) + if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0])) + return nil, block, nil # +1 + end + opt = (val = parse_arg(val, &error))[1] # +2 + val = conv_arg(*val) # +3 + if opt and !arg + argv.shift # +4 + else + val[0] = nil # +5 + end + val # +6 +end +``` + +(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.) + +## Configuration + +`reek`'s `Too Many Statements` detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `max_statements` | integer | The maximum number of statements allowed in a method before a warning is issued. Defaults to 5. | + +`Too Many Statements`'s default configuration is: + +```yaml +--- +TooManyStatements: + enabled: true + exclude: + - initialize + max_statements: 5 +``` + +By default, `initialize` is not checked for length; any class's constructor can be as long as necessary. diff --git a/docs/Uncommunicative-Method-Name.md b/docs/Uncommunicative-Method-Name.md new file mode 100644 index 000000000..3a299b5a7 --- /dev/null +++ b/docs/Uncommunicative-Method-Name.md @@ -0,0 +1,24 @@ +# Uncommunicative Method Name + +## Introduction + +An `Uncommunicative Method Name` is a method name that doesn't communicate its intent well enough. + +Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names. + +## Current Support in reek + +`Uncommunicative Method Name` checks for: + +* 1-character names +* any name ending with a number +* camelCaseVariableNames + +## Configuration + +`reek`'s Uncommunicative Method Name detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `reject` | array of regular expressions | The set of regular expressions that `reek` uses to check for bad names. Defaults to `[/^[a-z]$/, /[0-9]$/, /[A-Z]/]`. | +| `accept` | array of strings or regular expressions | Name that will be accepted (not reported) even if they match one of the `reject` expressions. | \ No newline at end of file diff --git a/docs/Uncommunicative-Module-Name.md b/docs/Uncommunicative-Module-Name.md new file mode 100644 index 000000000..0d7390135 --- /dev/null +++ b/docs/Uncommunicative-Module-Name.md @@ -0,0 +1,23 @@ +# Uncommunicative Module Name + +## Introduction + +An `Uncommunicative Module Name` is a module name that doesn't communicate its intent well enough. + +Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names. + +## Current Support in reek + +`Uncommunicative Module Name` checks for: + +* 1-character names +* any name ending with a number + +## Configuration + +`reek`'s `Uncommunicative Module Name` detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `reject` | array of regular expressions | The set of regular expressions that `reek` uses to check for bad names. Defaults to `[/^.$/, /[0-9]$/]`. | +| `accept` | array of strings or regular expressions | Name that will be accepted (not reported) even if they match one of the `reject` expressions. Defaults to `['Inline::C']`.| \ No newline at end of file diff --git a/docs/Uncommunicative-Name.md b/docs/Uncommunicative-Name.md new file mode 100644 index 000000000..df0e7f234 --- /dev/null +++ b/docs/Uncommunicative-Name.md @@ -0,0 +1,16 @@ +# Uncommunicative Name + +## Introduction + +An `Uncommunicative Name` is a name that doesn't communicate its intent well enough. + +Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names. + +## Current Support in Reek + +`reek` offers four related checks: + +* [Uncommunicative Method Name](Uncommunicative-Method-Name.md) +* [Uncommunicative Module Name](Uncommunicative-Module-Name.md) +* [Uncommunicative Parameter Name](Uncommunicative-Parameter-Name.md) +* [Uncommunicative Variable Name](Uncommunicative-Variable-Name.md) \ No newline at end of file diff --git a/docs/Uncommunicative-Parameter-Name.md b/docs/Uncommunicative-Parameter-Name.md new file mode 100644 index 000000000..ff1c2ca36 --- /dev/null +++ b/docs/Uncommunicative-Parameter-Name.md @@ -0,0 +1,24 @@ +# Uncommunicative Parameter Name + +## Introduction + +An `Uncommunicative Parameter Name` is a parameter name that doesn't communicate its intent well enough. + +Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names. + +## Current Support in reek + +`Uncommunicative Parameter Name` checks for: + +* 1-character names +* any name ending with a number +* camelCaseVariableNames + +## Configuration + +`reek`'s Uncommunicative Parameter Name detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `reject` | array of regular expressions | The set of regular expressions that `reek` uses to check for bad names. Defaults to `[/^.$/, /[0-9]$/, /[A-Z]/]@. | +| `accept` | array of strings or regular expressions | Name that will be accepted (not reported) even if they match one of the `reject` expressions. | \ No newline at end of file diff --git a/docs/Uncommunicative-Variable-Name.md b/docs/Uncommunicative-Variable-Name.md new file mode 100644 index 000000000..f1822c680 --- /dev/null +++ b/docs/Uncommunicative-Variable-Name.md @@ -0,0 +1,24 @@ +# Uncommunicative Variable Name + +## Introduction + +An `Uncommunicative Variable Name` is a variable name that doesn't communicate its intent well enough. + +Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names. + +## Current Support in Reek + +`Uncommunicative Variable Name` checks for: + +* 1-character names +* any name ending with a number +* camelCaseVariableNames + +## Configuration + +`reek`'s `Uncommunicative Variable Name` detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `reject` | array of regular expressions | The set of regular expressions that `reek` uses to check for bad names. Defaults to `[/^.$/, /[0-9]$/, /[A-Z]/]`. | +| `accept` | array of strings or regular expressions | Name that will be accepted (not reported) even if they match one of the `reject` expressions. Defaults to @['_']@.| \ No newline at end of file diff --git a/docs/Unused-Parameters.md b/docs/Unused-Parameters.md new file mode 100644 index 000000000..9f1b52f31 --- /dev/null +++ b/docs/Unused-Parameters.md @@ -0,0 +1,27 @@ +## Introduction + +`Unused Parameter` refers to methods with parameters that are unused in scope of the method. + +Having unused parameters in a method is code smell because leaving dead code in a method can never improve the method and it makes the code confusing to read. + +## Example + +Given: + +```Ruby +class Klass + def unused_parameters(x,y,z) + puts x,y # but not z + end +end +``` + +`reek` would emit the following warning: + +``` +[2]:Klass#unused_parameters has unused parameter 'z' (UnusedParameters) +``` + +## Configuration + +`Unused Parameter` offers the [Basic Smell Options](Basic-Smell-Options.md). \ No newline at end of file diff --git a/docs/Utility-Function.md b/docs/Utility-Function.md new file mode 100644 index 000000000..ccf7a0a98 --- /dev/null +++ b/docs/Utility-Function.md @@ -0,0 +1,46 @@ +# Utility Function + +## Introduction + +A Utility Function is any instance method that has no dependency on the state of the instance. + +A Utility Function reduces the code’s ability to communicate intent: code that “belongs” on one class but which is located in another can be hard to find, and may upset the “System of Names” in the host class. A Utility Function also affects the design’s flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application’s domain, and creates a loss of cohesion in the unwilling host class. + +A Utility Function often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there. + +## Example + +Given + +```Ruby +class UtilityFunction + def showcase(argument) + argument.to_s + argument.to_i + end +end +``` + +`reek` would report: + +``` +test.rb -- 2 warnings: + [2]:UtilityFunction#showcase doesn't depend on instance state (UtilityFunction) +``` + +## Current Support in reek + +`Utility Function` will warn about any method that: + +* is non-empty +* does not override an inherited method +* calls at least one method on another object +* doesn't use any of self's instance variables +* doesn't use any of self's methods + +## Configuration + +`reek`'s `Utility Function` detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus: + +| Option | Value | Effect | +| ---------------|-------------|---------| +| `max_helper_calls` | integer | The maximum number of method calls to other objects allowed within a method. Defaults to 2. | diff --git a/docs/Working-with-Rails.md b/docs/Working-with-Rails.md new file mode 100644 index 000000000..d72bd9b43 --- /dev/null +++ b/docs/Working-with-Rails.md @@ -0,0 +1,7 @@ +# Working with Rails + +With current versions of `reek` it's best to examine only your `app/models` folder, because `reek` raises false positives against views and controllers. + +For example, `params` is a kind of DTO (data transfer object) close to the system boundary, and so its characteristics should be different than regular code. But Reek doesn't know that (yet); `reek` thinks that all those `params[:something]` calls are a problem, and reports them as smells. + +We plan to improve Reek in the near future so that it plays better with Rails. For now though, your best bet is to restrict it to looking at `app/models` (and maybe `app/helpers` and `lib`). diff --git a/docs/YAML-Reports.md b/docs/YAML-Reports.md new file mode 100644 index 000000000..a2fdef1da --- /dev/null +++ b/docs/YAML-Reports.md @@ -0,0 +1,111 @@ +# YAML Reports + +## Introduction + +`Reek`'s `--yaml` option writes on $stdout a YAML dump of the smells found. Each reported smell has a number of standard fields and a number of fields that are specific to the smell's type. The common fields are as follows: + +| Field | Type | Value | +| ---------------|-------------|---------| +| source | string | The name of the source file containing the smell, or `$stdin` | +| lines | array | The source file line number(s) that contribute to this smell | +| context | string | The name of the class, module or method containing the smell | +| class | string | The class to which this smell belongs | +| subclass | string | This smell's subclass within the above class | +| message | string | The message that would have been printed in a standard Reek report | +| is_active | boolean | `false` if the smell is masked by a config file; `true` otherwise | + +All of these fields are grouped into hashes `location`, `smell` and `status` (see the examples below). + +## Examples + +Duplication: + +
+- !ruby/object:Reek::SmellWarning 
+  location: 
+    source: spec/samples/masked/dirty.rb
+    lines: 
+    - 5
+    - 7
+    context: Dirty#a
+  smell: 
+    class: Duplication
+    subclass: DuplicateMethodCall
+    occurrences: 2
+    call: puts(@s.title)
+    message: calls puts(@s.title) twice
+  status:
+    is_active: true
+
+ +[Nested Iterators](Nested-Iterators.md): + +
+- !ruby/object:Reek::SmellWarning 
+  location: 
+    source: spec/samples/masked/dirty.rb
+    lines: 
+    - 5
+    context: Dirty#a
+  smell: 
+    class: NestedIterators
+    subclass: ""
+    depth: 2
+    message: contains iterators nested 2 deep
+  status:
+    is_active: true
+
+ +[Uncommunicative Method Name](Uncommunicative-Method-Name.md): + +
+- !ruby/object:Reek::SmellWarning 
+  location: 
+    source: spec/samples/masked/dirty.rb
+    lines: 
+    - 3
+    context: Dirty#a
+  smell: 
+    class: UncommunicativeName
+    subclass: UncommunicativeMethodName
+    method_name: a
+    message: has the name 'a'
+  status:
+    is_active: false
+
+ +[Uncommunicative Variable Name](Uncommunicative-Variable-Name.md): + +
+- !ruby/object:Reek::SmellWarning 
+  location: 
+    source: spec/samples/masked/dirty.rb
+    lines: 
+    - 5
+    context: Dirty#a
+  smell: 
+    class: UncommunicativeName
+    subclass: UncommunicativeVariableName
+    variable_name: x
+    message: has the variable name 'x'
+  status:
+    is_active: true
+
+ +[Control Couple](Control-Couple.md): + +
+- !ruby/object:Reek::SmellWarning 
+  location: 
+    source: $stdin
+    lines: 
+    - 2
+    context: Turn#fred
+  smell: 
+    class: ControlCouple
+    subclass: BooleanParameter
+    parameter: arg
+    message: has boolean parameter 'arg'
+  status:
+    is_active: true
+