Skip to content

ran cargo clippy --fix -- -Wclippy::use_self #1165

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 7 additions & 1 deletion src/apply.rs
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation of this PR?

Copy link
Author

Choose a reason for hiding this comment

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

Are you taking about the default impl? Or the whole thing?

Copy link
Member

Choose a reason for hiding this comment

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

This description of this PR is missing, which I was looking for.

Copy link
Author

Choose a reason for hiding this comment

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

The purpose is to make the codebase idiomatic.

Copy link
Member

Choose a reason for hiding this comment

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

Note that clippy::use_self is under the nursery lint group. I would not recommend enabling it at this moment.

Also, personally I am not sure it is more idiomatic or not. Sometimes I found it hard to understand the surrounding context because we don't have type name explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Also, this adds lots of git diff / code churns.

Copy link
Author

Choose a reason for hiding this comment

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

I did not enable it, I just ran it.

It is more idiomatic, explicit types require you to read the type to make sure it's the same.

Copy link
Member

Choose a reason for hiding this comment

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

Good point.

Since the use_self lint introduces a lot of line changes, could you make it a separate commit to make other lint fix stand out? That helps the review of some lints get false positive fixes.

Also, one CI job is failing.

Copy link
Author

Choose a reason for hiding this comment

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

ok #1166

Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ extern "C" fn hunk_cb_c(hunk: *const raw::git_diff_hunk, data: *mut c_void) -> c
.unwrap_or(-1)
}

impl<'cb> Default for ApplyOptions<'cb> {
fn default() -> Self {
Self::new()
}
}

impl<'cb> ApplyOptions<'cb> {
/// Creates a new set of empty options (zeroed).
pub fn new() -> Self {
Expand All @@ -102,7 +108,7 @@ impl<'cb> ApplyOptions<'cb> {
}

fn flag(&mut self, opt: raw::git_apply_flags_t, val: bool) -> &mut Self {
let opt = opt as u32;
let opt = opt;
if val {
self.raw.flags |= opt;
} else {
Expand Down
38 changes: 19 additions & 19 deletions src/blame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl<'repo> Blame<'repo> {
}

impl<'blame> BlameHunk<'blame> {
unsafe fn from_raw_const(raw: *const raw::git_blame_hunk) -> BlameHunk<'blame> {
unsafe fn from_raw_const(raw: *const raw::git_blame_hunk) -> Self {
BlameHunk {
raw: raw as *mut raw::git_blame_hunk,
_marker: marker::PhantomData,
Expand Down Expand Up @@ -160,7 +160,7 @@ impl<'blame> BlameHunk<'blame> {

/// Returns number of lines in this hunk.
pub fn lines_in_hunk(&self) -> usize {
unsafe { (*self.raw).lines_in_hunk as usize }
unsafe { (*self.raw).lines_in_hunk }
}
}

Expand All @@ -172,7 +172,7 @@ impl Default for BlameOptions {

impl BlameOptions {
/// Initialize options
pub fn new() -> BlameOptions {
pub fn new() -> Self {
unsafe {
let mut raw: raw::git_blame_options = mem::zeroed();
assert_eq!(
Expand All @@ -184,7 +184,7 @@ impl BlameOptions {
}
}

fn flag(&mut self, opt: u32, val: bool) -> &mut BlameOptions {
fn flag(&mut self, opt: u32, val: bool) -> &mut Self {
if val {
self.raw.flags |= opt;
} else {
Expand All @@ -194,69 +194,69 @@ impl BlameOptions {
}

/// Track lines that have moved within a file.
pub fn track_copies_same_file(&mut self, opt: bool) -> &mut BlameOptions {
pub fn track_copies_same_file(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_TRACK_COPIES_SAME_FILE, opt)
}

/// Track lines that have moved across files in the same commit.
pub fn track_copies_same_commit_moves(&mut self, opt: bool) -> &mut BlameOptions {
pub fn track_copies_same_commit_moves(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_TRACK_COPIES_SAME_COMMIT_MOVES, opt)
}

/// Track lines that have been copied from another file that exists
/// in the same commit.
pub fn track_copies_same_commit_copies(&mut self, opt: bool) -> &mut BlameOptions {
pub fn track_copies_same_commit_copies(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_TRACK_COPIES_SAME_COMMIT_COPIES, opt)
}

/// Track lines that have been copied from another file that exists
/// in any commit.
pub fn track_copies_any_commit_copies(&mut self, opt: bool) -> &mut BlameOptions {
pub fn track_copies_any_commit_copies(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_TRACK_COPIES_ANY_COMMIT_COPIES, opt)
}

/// Restrict the search of commits to those reachable following only
/// the first parents.
pub fn first_parent(&mut self, opt: bool) -> &mut BlameOptions {
pub fn first_parent(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_FIRST_PARENT, opt)
}

/// Use mailmap file to map author and committer names and email addresses
/// to canonical real names and email addresses. The mailmap will be read
/// from the working directory, or HEAD in a bare repository.
pub fn use_mailmap(&mut self, opt: bool) -> &mut BlameOptions {
pub fn use_mailmap(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_USE_MAILMAP, opt)
}

/// Ignore whitespace differences.
pub fn ignore_whitespace(&mut self, opt: bool) -> &mut BlameOptions {
pub fn ignore_whitespace(&mut self, opt: bool) -> &mut Self {
self.flag(raw::GIT_BLAME_IGNORE_WHITESPACE, opt)
}

/// Setter for the id of the newest commit to consider.
pub fn newest_commit(&mut self, id: Oid) -> &mut BlameOptions {
pub fn newest_commit(&mut self, id: Oid) -> &mut Self {
unsafe {
self.raw.newest_commit = *id.raw();
}
self
}

/// Setter for the id of the oldest commit to consider.
pub fn oldest_commit(&mut self, id: Oid) -> &mut BlameOptions {
pub fn oldest_commit(&mut self, id: Oid) -> &mut Self {
unsafe {
self.raw.oldest_commit = *id.raw();
}
self
}

/// The first line in the file to blame.
pub fn min_line(&mut self, lineno: usize) -> &mut BlameOptions {
pub fn min_line(&mut self, lineno: usize) -> &mut Self {
self.raw.min_line = lineno;
self
}

/// The last line in the file to blame.
pub fn max_line(&mut self, lineno: usize) -> &mut BlameOptions {
pub fn max_line(&mut self, lineno: usize) -> &mut Self {
self.raw.max_line = lineno;
self
}
Expand All @@ -265,7 +265,7 @@ impl BlameOptions {
impl<'repo> Binding for Blame<'repo> {
type Raw = *mut raw::git_blame;

unsafe fn from_raw(raw: *mut raw::git_blame) -> Blame<'repo> {
unsafe fn from_raw(raw: *mut raw::git_blame) -> Self {
Blame {
raw,
_marker: marker::PhantomData,
Expand All @@ -286,7 +286,7 @@ impl<'repo> Drop for Blame<'repo> {
impl<'blame> Binding for BlameHunk<'blame> {
type Raw = *mut raw::git_blame_hunk;

unsafe fn from_raw(raw: *mut raw::git_blame_hunk) -> BlameHunk<'blame> {
unsafe fn from_raw(raw: *mut raw::git_blame_hunk) -> Self {
BlameHunk {
raw,
_marker: marker::PhantomData,
Expand All @@ -301,8 +301,8 @@ impl<'blame> Binding for BlameHunk<'blame> {
impl Binding for BlameOptions {
type Raw = *mut raw::git_blame_options;

unsafe fn from_raw(opts: *mut raw::git_blame_options) -> BlameOptions {
BlameOptions { raw: *opts }
unsafe fn from_raw(opts: *mut raw::git_blame_options) -> Self {
Self { raw: *opts }
}

fn raw(&self) -> *mut raw::git_blame_options {
Expand Down
8 changes: 4 additions & 4 deletions src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'repo> Blob<'repo> {
impl<'repo> Binding for Blob<'repo> {
type Raw = *mut raw::git_blob;

unsafe fn from_raw(raw: *mut raw::git_blob) -> Blob<'repo> {
unsafe fn from_raw(raw: *mut raw::git_blob) -> Self {
Blob {
raw,
_marker: marker::PhantomData,
Expand Down Expand Up @@ -108,7 +108,7 @@ impl<'repo> BlobWriter<'repo> {
impl<'repo> Binding for BlobWriter<'repo> {
type Raw = *mut raw::git_writestream;

unsafe fn from_raw(raw: *mut raw::git_writestream) -> BlobWriter<'repo> {
unsafe fn from_raw(raw: *mut raw::git_writestream) -> Self {
BlobWriter {
raw,
need_cleanup: true,
Expand Down Expand Up @@ -139,12 +139,12 @@ impl<'repo> io::Write for BlobWriter<'repo> {
if let Some(f) = (*self.raw).write {
let res = f(self.raw, buf.as_ptr() as *const _, buf.len());
if res < 0 {
Err(io::Error::new(io::ErrorKind::Other, "Write error"))
Err(io::Error::other("Write error"))
} else {
Ok(buf.len())
}
} else {
Err(io::Error::new(io::ErrorKind::Other, "no write callback"))
Err(io::Error::other("no write callback"))
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<'repo> Branch<'repo> {
}

/// Move/rename an existing local branch reference.
pub fn rename(&mut self, new_branch_name: &str, force: bool) -> Result<Branch<'repo>, Error> {
pub fn rename(&mut self, new_branch_name: &str, force: bool) -> Result<Self, Error> {
let mut ret = ptr::null_mut();
let new_branch_name = CString::new(new_branch_name)?;
unsafe {
Expand Down Expand Up @@ -100,7 +100,7 @@ impl<'repo> Branch<'repo> {

/// Return the reference supporting the remote tracking branch, given a
/// local branch reference.
pub fn upstream(&self) -> Result<Branch<'repo>, Error> {
pub fn upstream(&self) -> Result<Self, Error> {
let mut ret = ptr::null_mut();
unsafe {
try_call!(raw::git_branch_upstream(&mut ret, &*self.get().raw()));
Expand Down Expand Up @@ -129,7 +129,7 @@ impl<'repo> Branches<'repo> {
///
/// This function is unsafe as it is not guaranteed that `raw` is a valid
/// pointer.
pub unsafe fn from_raw(raw: *mut raw::git_branch_iterator) -> Branches<'repo> {
pub unsafe fn from_raw(raw: *mut raw::git_branch_iterator) -> Self {
Branches {
raw,
_marker: marker::PhantomData,
Expand Down
12 changes: 6 additions & 6 deletions src/buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Default for Buf {

impl Buf {
/// Creates a new empty buffer.
pub fn new() -> Buf {
pub fn new() -> Self {
crate::init();
unsafe {
Binding::from_raw(&mut raw::git_buf {
Expand All @@ -37,27 +37,27 @@ impl Buf {
///
/// Returns `None` if the buffer is not valid utf-8.
pub fn as_str(&self) -> Option<&str> {
str::from_utf8(&**self).ok()
str::from_utf8(self).ok()
}
}

impl Deref for Buf {
type Target = [u8];
fn deref(&self) -> &[u8] {
unsafe { slice::from_raw_parts(self.raw.ptr as *const u8, self.raw.size as usize) }
unsafe { slice::from_raw_parts(self.raw.ptr as *const u8, self.raw.size) }
}
}

impl DerefMut for Buf {
fn deref_mut(&mut self) -> &mut [u8] {
unsafe { slice::from_raw_parts_mut(self.raw.ptr as *mut u8, self.raw.size as usize) }
unsafe { slice::from_raw_parts_mut(self.raw.ptr as *mut u8, self.raw.size) }
}
}

impl Binding for Buf {
type Raw = *mut raw::git_buf;
unsafe fn from_raw(raw: *mut raw::git_buf) -> Buf {
Buf { raw: *raw }
unsafe fn from_raw(raw: *mut raw::git_buf) -> Self {
Self { raw: *raw }
}
fn raw(&self) -> *mut raw::git_buf {
&self.raw as *const _ as *mut _
Expand Down
Loading
Loading