hpi-swa-teaching/swalint

Code Critics Tool for Squeak

SwaLint

A code critics tool for Smalltalk projects. After installation you can open the SwaLint Code Critics via Apps.

Please visit the SwaLint wiki for further information, future plans and technical advice.

Installation

Latest release

There are two ways of installing the latest release. You can either use a SAR file or Metacello.

SAR file

Go to the latest release of SwaLint, download the .sar file, drag and drop it into your image and choose "install SAR".

Metaclleo

1. Make sure you have metacello installed.
2. Load the project with (it might ask you to click "Proceed" several times):

[Metacello new
  baseline: 'SwaLint';
  repository: 'github://hpi-swa-teaching/SwaLint:release/packages';
  get;
  load: #default]
    on: MetacelloSkipDirtyPackageLoad
    do: [:e | e resume: false]

Latest develop version

Metaclleo

1. Make sure you have metacello installed.
2. Load the project with (it might ask you to click "Proceed" several times):

[Metacello new
  baseline: 'SwaLint';
  repository: 'github://hpi-swa-teaching/SwaLint:develop/packages';
  get;
  load: #default]
    on: MetacelloSkipDirtyPackageLoad
    do: [:e | e resume: false]

swalint's Issues

Instance variables defined in all subclasses not working properly

This LinterTest does not do what it should.
Accepted when test succeeds.

"A + B * C" might be "A + (B * C)" not detecting division

It does not recognize A+B/C.
Add a test and implement it.

Dead code rework

Currently, dead code is a duplicate of smallLintClassNotReferenced.
Instead of removing it, it would be nice to detect the following code as dead code:
"...
false ifTrue: [someDeadCode].
true ifTrue: [someOtherDeadCode]."
Write tests and implement it.

ERROR'ed tests with swalint 4.0 when running against Pheno

• possible occurence of a+b*c ... in e.g. PHIconExample, PHDropdownExample
• complex methods in e.g. PHIcon, PHBoxLayout, PHQuiz
• dot after return in e.g. PHIcon, PHPresentation, PHQuiz
• long methods (without empty lines) in the same as above
• ... and actually quite a bit more. I guess some utility just doesn't like something, so everything that uses the utility keeps failing for the same classes

Uses do: instead of collect: or select:''s could be better

Currently, it does find "add: each" but not "add: (5+each)".
Test already existing, but commented out. Comment it in, please.
Enhance the LinterTest to make the test green. A way of doing this could be ignoring special Characters (Character isSpecial: +-*/) as message selectors.

Heading of selection browser is set wrong

Heading of selection browser is "Issues for selected tests on selected objects [X]" when you open it. After clicking on an other method the heading is "Implementors of X"

Duplication of LinterTests

Redundant code
Missing class comments
Unclassified methods
Long or complex methods

Are all duplications or have duplications.

Index of existing Tests [24]

As a new user, I’d like to be able to view the criteria for a test, or at least a summary/in-depth description, so I'll know exactly what to look out for and how to fix errors.

In the first week, all tests with the initial A to L are tackled.

• provide a short summary text
• classify based on artifact level (works on method/methods/classes/categories/packages)
• decide for a relevancy (if applicable) in e.g. important/nice-to-have/minor
• decide if an automatic fix can be provided
• check if test works as intended

Error: symbols can not be modified

When running a yet not more specified combination of modules and tests, SWALint errors with an "Error: symbols can not be modified", see image below

It looks like the errors occur in the RB classes, so it might be another problem with the refactoring browser which could not be tackled here,

Class-based SmallLint tests are executed per method

Class-based SmallLint tests (and potentially other tests, but their behavior has not been examined yet) seem to be executed exactly as often per class as the amount of methods in the class dictates. That also means a test ist never executed if there are no methods in a class.

This suggests that the current implementation just delegates calls arriving at checkMethod to checkClass, which would yield exactly this behavior.

Obviously, this cannot be intended and should thus be fixed.

Suggested Solutions

As a developer, I’d would like SwaLint to suggest solutions that I can accept or reject, in order to speed up development by reducing workload -- especially for 'mundane' tests.

Linter tests not listed in linter result screen

The results of some linter tests seem not be to be listed in the result screen of the linter. Especially the tests "Class variable capitalization" (line 14), "Instance variable capitalization" (l. 22), "No class comment" (l. 72), "Variables not referenced" (l. 86) and "Has class instance variables but no initialize method" (l. 13) seem to be affected.
(At least) they share the attribute that they do not have the basic returnClass for the result, their types are either RBClassEnviroment or RBVariableEnviroment,

No class comment does not register result

Currently it does register the result (although it is computed???). There are similar problems in other related tests, something with Environments?
Other LinterTests: Variables not referenced.
Ask Björn.

Scrolling with arrow keys in linter test results [8]

As a user I would like to be able to scroll through the linter test results with arrow keys to be able to have a more precise look on the results.

Compare Errors with Previous Iterations, Statistics

As a manager, I’d like to compare multiple test runs with each other, so I can see how the code base developed in terms of code smells and potentially find trouble spots.

Error: Attempt to index a non-existent element in an OrderedCollection

Try to do the following to reproduce the error:

1. Open a fresh Linter (SwaLint reset open).
2. Select all test categories (right upper window).
3. Select the magic numbers test (right lower window).
4. Unselect the "Software System Metrics" test category.

Add Tests for Your Own Code

As a developer, I’d like to be able to add tests for my own code, so I have the option to softly enforce its usage. Ideally, these tests can be distributed with my code.

Ordering and Grouping of linter tests

As a SWA student or at least someone trying to write good software but not being into testing that deep I want to have the liter tests grouped or ordered in a way that I can easily find "easy" and intuitively useful tests, so that I do not have to be browse threw all different categories and tests and waste a lot of time and enjoyment with SWALint.

Selection of multiple tests

When using "click and swipe" to select multiple tests it doesn't select every test when you scroll to fast

Lint test failure markers in system browser

As a developer, I'd like to see markers in the system browser hinting at the places where linter tests have gone off. Moreover, when hovering over such a marker, a tooltip should show up and display which linter test(s) has/have failed there.

In combination with #14, short descriptions or summaries of failed linter tests could be displayed inside the tooltip.
In combination with #19, these markers could update automatically when the user changes code.
In combination with #15, suggested solutions for the problem could be displayed in each tooltip.

Error: SLPragmaPreference does not understand #selectors [5]

If navigate to the options, click a preference, show details and then click on "more", the error will occur.

Unnecessary size check missing some cases

Description says: "Check for code that checks that a collection is non-empty before sending it an iteration message (e.g., do:, collect:, etc.). Since the collection iteration messages work for empty collections, we do not need to clutter up our method with the extra size check.".
But it does not find "collection isEmpty not ifFalse: [collection do: [:each | ...]]"
Enhance it. Test is existing, maybe enhance it further!?

Fix the following linter tests

Referenced by their table entry:

• 35
• 42
• 47
• 57
• 64
• 78
• 86
• 100
• 106
• 112
• 114
• 120
• 121
• 124

UI: Click selects multiple items from

Sometimes when trying to select one item from a list in the user interface, the linter will select another item too. If you try again, the same misbehaviour will continue.
Since we do not know how to reproduce it, we cannot test it. So find out how to reproduce it first.

Errors in test "Significant Duplication"

The test "Significant Duplication" produces a "ERROR" for each tested method.

Button for SwaLint reset

It would be nice to have a button to reset SwaLint. If you get a new version of SwaLint, you might need to reset it. If there's no button, you'll forget and things won't work.

Redundant class name in selector

This does not check every class. It checks for isMeta, which might be the reason.
Write a test (with example from test description) and make it work.

Deprecate old Buttons [7]

• find buttons that currently produce errors
• find buttons that currently produce results that don't make sense
• explain their original purpose
• argue why they can be removed/how they can be fixed

Subclass responsibility not defined not working

Currently, it does not work, should do: "Checks that all subclassResponsibility
methods are defined in all leaf classes."
Write test coverage for it and fix it.

Using deprecated methods

Sometimes you call methods, that are deprecated, but are unaware of it. You can enable deprecation warnings, but they will only trigger if that path of code is executed.
We could check if the code does directly call a method that is marked as deprecated (which is not perfect, but does not require actually executing that path of code).
There is already a similar LinterTest, but it seems to be related to Pharao and I do not understand what it is actually doing. What are wellKnownClasses?

Fix broken testing frameworks

The import in the DevelopBasedOn2015 branch (https://github.com/hpi-swa-teaching/SwaLint/tree/developBasedOn2015) fixes the UI errors but does not work with the actual testing framework. That has to be fixed.

Magic numbers test "Error: symbols can not be modified"

At least with the magic numbers test (but most likely with few others as well) you get the "Error: symbols can not be modified" when trying to lint class with methods with primitives in it.

Shortcuts

As a seasoned user, I’d like to be able to use keyboard shortcuts instead of the GUI, so I don’t have to navigate through context menus and can get back to coding even faster.

Unnecessary "search results" category in the options [2]

The reference browser has an unused category "search results" which does not seem necessary. There also exists the "--search results--" category which correctly shows the results of a search

Spell Checks not working

The following LinterTests related to spelling are not working:
Class categories
Class comments
Class names
Class variable names
There seem to be issues with normalization.

Make linter non-blocking

As a user running multiple and long tests, I would like to be able to still work in my image while linter tests are going on to increase my productivity.
At least I would like to be able to cancel the linting.

Instance variable capitalization not working properly

This LinterTest does not do what it should. Does not actually call (any?) rule.
Fix it!

Long Methods counting empty lines

Since there is already a LinterTest, that does the same but without emtpy lines, either remove this test or update name + description.

Make a uniform way of linter test implementation

As a user, that wants to define customized linter tests, I would like to have a clear interface for linter test , I would like to add, to be able to add my own linting rules.
Therefore the test interface of the SwaLint-Plugins classes should be unified (the SmallLint test use a different way, then the other classes (they address RB classes)) and not need a registration of every part of the test.

Complex Method errors, example in description

wrap
	^#word_char

Clean up test enviroment

It seems like the combination of our test environment (abstract test classes with inherited tests) results in different test results:

• It seems like the normal test runner counts the test correctly
• The auto test runner definitely counts to many tests (each test is done either two, three or four times)
• Travis does not care, if we add new tests

Fix magic numbers linter test results [11]

As a user I would like to have the magic numbers test fixed to be able to review my code based on this linter test results.
The current result looks as follows (some of them fail, some go threw)

Magic Numbers testing [15]

Currently, the magic numbers detection does not do, what one thinks it should do:
"b := 'hrm' " -> There's one magic number ('hrm').
"b := 'hello'" -> There's no magic number.
"b := 'abc'" -> There's no magic number.
"b := 'abf'" -> There's one magic number.
"b := 'LarsSeifert'" -> There's one magic number.
"b := 'Lars Seifert' -> There's no magic number.
"b := 3.1415931234 -> There's no magic number.

So, "magic numbers" are wrongly spelled or badly formatted strings.
It would be better if magic numbers would do what we expect (b := 3.141...) and nothing else.

Cancel linting

As a user having many tests and a huge code base that started the linting by mistake, which runs very long, I would like to be able to cancel the linting.

Error: MessageNotUnderstood False >> sort [5]

When going into the Linter Options/Preferences and clicking at "--all--", an error window appears. Further clicks on other categories will then produce the same error too.
That's not good.

Reject code changes (linter integration at SmalltalkCi) when they do not meet the predefined coding standards

As a development that wants to have fix coding standards for their team, we would like the linter SmalltalkCI integration to check for a defined set of tests (the coding standards) and reject the code change if it does not meet them, to improve our code quality. It would be great if that is more than just "Dot after return consistency".
Therefore we should also think about a group of tests which are only responsible for those coding standards.

isList fails on travis

Find another way to do the same thing that does not use dependencies.
Completed when travis is green again.

Continuous linting

As a developer, I'd like the linter to continuously lint my code and show the results when some linter tests fail, i.e., in a continuously open status window. This way, I don't need to run the linter over and over again manually. Instead, I immediately see when a linter test goes off and can fix the problem just after I introduced it.

SLWrongTypeError when using test "dead code" [3]

We're getting a SLWrongTypeError when using the test "dead code" on our project code.

Literal values and magic numbers do the same

After having "fixed" #6, that results in non-failing literal values and method selectors tests, it seems like the liter values and the magic numbers test do the same. Have a look at the output of the linter of only running the literal values and the magic numbers test

all results doubled and in one category.
However, when trying to browse the result, the same problem as mentioned in #1 (but obviously not dead code, but magic numbers as the second word) is raised.