diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a8cc60e368..a7477a9636 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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 where @@ -36,7 +39,7 @@ pub struct X 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 where @@ -46,8 +49,7 @@ pub struct X } ``` -- 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 } ``` -- 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 // <- 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 { phantom: PhanomData, @@ -100,7 +92,38 @@ pub struct X { impl X {} // <- 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 // <- 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(&self, em: &EM) + where + EM: EventManager + { + em.do_stuff(self); + } +} + +pub trait EventManager: Sized { + fn do_stuff(&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 {}; // <- Generics are alphabetically ordered ``` @@ -108,7 +131,8 @@ But not, ```rust pub struct X {}; // <- 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 {}