Update CONTRIBUTING.md to forbid cyclic dependency (#2999)
This commit is contained in:
parent
ae3ea23607
commit
91c46d1a33
@ -2,7 +2,7 @@
|
||||
|
||||
For bugs, feel free to open issues or contact us directly. Thank you for your support. <3
|
||||
|
||||
## Pull Request guideline
|
||||
## Pull Request Guideline
|
||||
|
||||
Even though we will gladly assist you in finishing up your PR, try to:
|
||||
|
||||
@ -14,7 +14,7 @@ Even though we will gladly assist you in finishing up your PR, try to:
|
||||
|
||||
Some of the parts in this list may be hard, don't be afraid to open a PR if you cannot fix them by yourself, so we can help.
|
||||
|
||||
### Pre-commit hooks
|
||||
### Pre-commit Hooks
|
||||
|
||||
Some of these checks can be performed automatically during commit using [pre-commit](https://pre-commit.com/).
|
||||
Once the package is installed, simply run `pre-commit install` to enable the hooks, the checks will run automatically before the commit becomes effective.
|
||||
@ -27,7 +27,10 @@ Before making your pull requests, try to see if your code follows these rules.
|
||||
- `PhantomData` should have the smallest set of types needed. Try not adding `PhantomData` to your struct unless it is really necessary. Also even when you really need `PhantomData`, try to keep the types `T` used in `PhantomData` as smallest as possible
|
||||
- Wherever possible, trait implementations with lifetime specifiers should use '_ lifetime elision.
|
||||
- Complex constructors should be replaced with `typed_builder`, or write code in the builder pattern for yourself.
|
||||
- Remove generic restrictions at the definitions (e.g., we do not need to specify that types impl `Serialize`, `Deserialize`, or `Debug` anymore at the struct definitions). Therefore, try avoiding code like this unless the constraint is really necessary.
|
||||
|
||||
|
||||
## Rules for Generics and Associated Types
|
||||
1. Remove generic restrictions at the definitions (e.g., we do not need to specify that types impl `Serialize`, `Deserialize`, or `Debug` anymore at the struct definitions). Therefore, try avoiding code like this unless the constraint is really necessary.
|
||||
```rust
|
||||
pub struct X<A>
|
||||
where
|
||||
@ -36,7 +39,7 @@ pub struct X<A>
|
||||
fn ...
|
||||
}
|
||||
```
|
||||
- Reduce generics to the least restrictive necessary. __Never overspecify the constraints__. There's no automated tool to check the useless constraints, so you have to verify this manually.
|
||||
2. Reduce generics to the least restrictive necessary. __Never overspecify the constraints__. There's no automated tool to check the useless constraints, so you have to verify this manually.
|
||||
```rust
|
||||
pub struct X<A>
|
||||
where
|
||||
@ -46,8 +49,7 @@ pub struct X<A>
|
||||
}
|
||||
```
|
||||
|
||||
- Prefer generic to associated types in traits definition as much as possible. They are much easier to use around, and avoid tricky caveats / type repetition in the code. It is also much easier to have unconstrained struct definitions.
|
||||
|
||||
3. Prefer generic to associated types in traits definition as much as possible. They are much easier to use around, and avoid tricky caveats / type repetition in the code. It is also much easier to have unconstrained struct definitions.
|
||||
Try not to write this:
|
||||
```rust
|
||||
pub trait X
|
||||
@ -65,7 +67,7 @@ pub trait X<A>
|
||||
}
|
||||
```
|
||||
|
||||
- Traits which have an associated type (if you have made sure you cannot use a generic instead) should refer to the associated type, not the concrete/generic. In other words, you should only have the associated type when you can define a getter to it. For example, in the following code, you can define a associate type.
|
||||
4. Traits which have an associated type (if you have made sure you cannot use a generic instead) should refer to the associated type, not the concrete/generic. In other words, you should only have the associated type when you can define a getter to it. For example, in the following code, you can define a associate type.
|
||||
```rust
|
||||
pub trait X
|
||||
{
|
||||
@ -74,17 +76,7 @@ pub trait X
|
||||
fn a(&self) -> Self::A;
|
||||
}
|
||||
```
|
||||
|
||||
- __Ideally__ the types used in the arguments of methods in traits should have the same as the types defined on the traits.
|
||||
```rust
|
||||
pub trait X<A, B, C> // <- this trait have 3 generics, A, B, and C
|
||||
{
|
||||
fn do_stuff(&self, a: A, b: B, c: C); // <- this is good because it uses all A, B, and C.
|
||||
|
||||
fn do_other_stuff(&self, a: A, b: B); // <- this is not ideal because it does not have C.
|
||||
}
|
||||
```
|
||||
- Generic naming should be consistent. Do NOT use multiple name for the same generic, it just makes things more confusing. Do:
|
||||
5. Generic naming should be consistent. Do NOT use multiple name for the same generic, it just makes things more confusing. Do:
|
||||
```rust
|
||||
pub struct X<A> {
|
||||
phantom: PhanomData<A>,
|
||||
@ -100,7 +92,38 @@ pub struct X<A> {
|
||||
|
||||
impl<B> X<B> {} // <- Do NOT do that, use A instead of B
|
||||
```
|
||||
- Always alphabetically order the type generics. Therefore,
|
||||
6. __Ideally__ the types used in the arguments of methods in traits should have the same as the types defined on the traits.
|
||||
```rust
|
||||
pub trait X<A, B, C> // <- this trait have 3 generics, A, B, and C
|
||||
{
|
||||
fn do_stuff(&self, a: A, b: B, c: C); // <- this is good because it uses all A, B, and C.
|
||||
|
||||
fn do_other_stuff(&self, a: A, b: B); // <- this is not ideal because it does not have C.
|
||||
}
|
||||
```
|
||||
7. Try to avoid cyclical dependency if possible. Sometimes it is necessary but try to avoid it. For example, The following code is a bad example.
|
||||
```rust
|
||||
pub struct X {}
|
||||
pub struct Y {}
|
||||
|
||||
pub trait Fuzzer: Sized {
|
||||
fn fuzz<EM>(&self, em: &EM)
|
||||
where
|
||||
EM: EventManager
|
||||
{
|
||||
em.do_stuff(self);
|
||||
}
|
||||
}
|
||||
|
||||
pub trait EventManager: Sized {
|
||||
fn do_stuff<Z>(&self, fuzzer: &Z); // <- This function signature should not take fuzzer
|
||||
}
|
||||
```
|
||||
trait `EventManager` should not implement any method that takes fuzzer, any object that could implement `Fuzzer` trait.
|
||||
|
||||
|
||||
## Formatting
|
||||
1. Always alphabetically order the type generics. Therefore,
|
||||
```rust
|
||||
pub struct X<E, EM, OT, S, Z> {}; // <- Generics are alphabetically ordered
|
||||
```
|
||||
@ -108,7 +131,8 @@ But not,
|
||||
```rust
|
||||
pub struct X<S, OT, Z, EM, E> {}; // <- Generics are not ordered
|
||||
```
|
||||
- Similarly, generic bounds in `where` clauses should be alphabetically sorted. Prefer:
|
||||
2. Similarly, generic bounds in `where` clauses should be alphabetically sorted.
|
||||
Prefer:
|
||||
```rust
|
||||
pub trait FooA {}
|
||||
pub trait FooB {}
|
||||
|
Loading…
x
Reference in New Issue
Block a user