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

Rust Clippy #1467

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ htmlcov
*.egg-info
.coverage.*
src
!tests/rust/test_*/src
site
.zanata-cache
*.exe
Expand Down
67 changes: 67 additions & 0 deletions bears/rust/RustClippyLintBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import json
import os

from coalib.bearlib.abstractions.Linter import linter
from coalib.results.Result import Result
from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY
from dependency_management.requirements.CargoRequirement import (
CargoRequirement)


@linter(executable='cargo',
global_bear=True,
use_stdout=False,
use_stderr=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you also need stdout? If not you should deactivate it with use_stdout=False (it's not automatically deactivated with use_stderr=True)

class RustClippyLintBear:
LANGUAGES = {'Rust'}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
CAN_DETECT = {
'Formatting',
'Unused Code',
'Syntax',
'Unreachable Code',
'Smell',
'Code Simplification',
}
EXECUTABLE = 'cargo.exe' if os.name == 'nt' else 'cargo'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be interested in https://gitlab.com/coala/package_manager/issues/55 , which would allow us to manage the dependency like we do with other languages, and the check_prerequisites would be unnecessary.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's interesting. Am I correct in assuming that the CargoDependency isn't yet released?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is released as part of dependency_management 0.4, and available to the bears; see coala/coala@79a4aad00f20a

REQUIREMENTS = {
CargoRequirement('clippy')
}
SEVERITY_MAP = {
'warning': RESULT_SEVERITY.NORMAL,
'error': RESULT_SEVERITY.MAJOR,
}

@staticmethod
def create_arguments(config_file):
args = ('clippy', '--quiet', '--color', 'never',
'--', '-Z', 'unstable-options',
'--error-format', 'json',
'--test')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to use @linter? IIRC it supports global bears by now :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to use @linter. Seems like it should work; except that it doesn't do JSON yet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. It doesn't do JSON out of the box, but output_format=None, lets one customize the behaviour. I'll make an issue about this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind: coala/coala#2072

return args

def process_output(self, output, filename, file):
# Rust outputs \n's, instead of the system default.
for line in output.split('\n'):
if not line:
continue
# cargo still outputs some text, even when in quiet mode,
# when a project does not build. We skip this, as the
# real reason will be reported on another line.
if line.startswith('To learn more, run the command again'):
continue
yield self.new_result(json.loads(line))

def new_result(self, issue):
span = issue['spans'][0]
return Result.from_values(
origin=self.__class__.__name__,
message=issue['message'],
file=span['file_name'],
line=span['line_start'],
end_line=span['line_end'],
column=span['column_start'],
end_column=span['column_end'],
severity=self.SEVERITY_MAP[issue['level']])
Empty file added bears/rust/__init__.py
Empty file.
2 changes: 2 additions & 0 deletions tests/rust/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
target
Cargo.lock
49 changes: 49 additions & 0 deletions tests/rust/RustClippyLintBearTest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import unittest
import os
from queue import Queue
from shutil import which
from unittest.case import skipIf

from coalib.settings.Section import Section

from bears.rust.RustClippyLintBear import RustClippyLintBear


@skipIf(which('cargo') is None, 'Cargo is not installed')
class RustClippyLintBearTest(unittest.TestCase):

def setUp(self):
self.section = Section('name')
self.queue = Queue()
self.file_dict = {}
self.uut = RustClippyLintBear(self.file_dict, self.section, self.queue)

def change_directory(self, directory_name):
test_path = os.path.abspath(os.path.join(os.path.dirname(__file__),
directory_name))
os.chdir(test_path)

def set_config_dir(self, directory):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a note here about the coala bug that this 'fixes'/workarounds

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if I'm gonna use the CargoRequirement, this is not needed anymore, I suppose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether CargoRequirement solves this problem. My gut feeling is that it doesnt.

# Work around https://github.com/coala/coala/issues/3867
test_path = os.path.abspath(os.path.join(
os.path.dirname(__file__), directory))
self.uut.get_config_dir = lambda *args, **kwargs: test_path

def test_ok_source(self):
self.set_config_dir('test_ok')
results = list(self.uut.run())
self.assertTrue(len(results) == 0)

def test_bad_source(self):
self.set_config_dir('test_bad')
results = list(self.uut.run())
self.assertTrue(len(results) >= 3)

def test_error_source(self):
self.set_config_dir('test_error')
results = list(self.uut.run())
self.assertTrue(len(results) == 1)

result = results[0]
print(result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

darn, spotted a print statement, I wanted to ack this!

self.assertTrue(result.message == 'mismatched types')
Empty file added tests/rust/__init__.py
Empty file.
5 changes: 5 additions & 0 deletions tests/rust/test_bad/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "coala_test_files"
version = "0.1.0"

[dependencies]
24 changes: 24 additions & 0 deletions tests/rust/test_bad/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This function should trigger unused code (warning level)
fn testssss() {
// This comparison should trigger a `deny` level lint
let mut x: f64 = 0.0;
if x == std::f64::NAN {
}
x += 1.; // Triggers allow style lint
println!("{}", x);
}


// We test whether bad code is found in (unit) tests
#[cfg(test)]
mod tests {
#[test]
fn it_works() {
let x = Some(1u8);
// This match triggers a `warning` level lint
match x {
Some(y) => println!("{:?}", y),
_ => ()
}
}
}
5 changes: 5 additions & 0 deletions tests/rust/test_error/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "coala_test_files"
version = "0.1.0"

[dependencies]
4 changes: 4 additions & 0 deletions tests/rust/test_error/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
// This does not compile. Message is 'mismatched types'
let a: i32 = "";
}
5 changes: 5 additions & 0 deletions tests/rust/test_ok/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "coala_test_files"
version = "0.1.0"

[dependencies]
3 changes: 3 additions & 0 deletions tests/rust/test_ok/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#[test]
fn it_works() {
}