Lint rust plugins #6950
Lint rust plugins #6950
Conversation
f624ae8 to
812b000
Compare
ee86a0c to
812b000
Compare
Hello! Could you please help me with local testing make check
sudo make install
sudo make lib389-installand then i run tests like this: sudo py.test -s dirsrvtests/tests/suites/entryuuid/
sudo py.test -s dirsrvtests/tests/suites/password/but the test doesn't seem to "see" my changes, I even added errors in code on purpose to see if something changes but it doesn't perhaps I'm missing something? thank you in advance |
|
Do you have any other versions of 389-ds-base installed? And then install the resulting rpms from |
I recently updated to debian 13, maybe I'll try to use repo from salsa and build debs from it |
|
In this case ensure that you don't have 389-ds packages installed from the distro, and use binaries installed in the prefix. |
0c556be to
ccf8ece
Compare
Hello! Are tests run automatically or manually? Locally issue seems to be solved but I want to double check with CI |
For the first-time contributors CI run needs to be approved. I've kicked off the tests. |
Got it, thanks |
ccf8ece to
f150996
Compare
|
@vashirov can you please push the tests again? |
Reviewer's GuideThis PR applies Clippy-driven lint and style updates across the Rust plugin crates, improving safety annotations, const-correctness, pointer handling, formatting, and match patterns without altering any functional behavior. Class diagram for updated pointer and const usage in plugin typesclassDiagram
class Pblock {
+raw_pb: *const libc::c_void
+new() Self
}
class PblockRef {
+raw_pb: *const libc::c_void
+new(raw_pb: *const libc::c_void) Self
+as_ptr() *const libc::c_void
}
Pblock --> PblockRef
class EntryRef {
+raw_e: *const libc::c_void
+new(raw_e: *const libc::c_void) Self
+get_sdnref() SdnRef
}
class SdnRef {
+raw_sdn: *const libc::c_void
+new(raw_sdn: *const libc::c_void) Self
+as_ptr() *const libc::c_void
}
EntryRef --> SdnRef
class ValueArrayRef {
+raw_slapi_val: *const *const slapi_value
+new(raw_slapi_val: *const libc::c_void) Self
+iter() ValueArrayRefIter
}
class ValueArrayRefIter {
+idx: usize
+va_ref: &ValueArrayRef
}
ValueArrayRef --> ValueArrayRefIter
class ValueRef {
+raw_slapi_val: *const slapi_value
+new(raw_slapi_val: *const libc::c_void) Self
+as_ptr() *const slapi_value
}
ValueArrayRefIter --> ValueRef
class Value {
+value: ValueRef
+take_ownership() *mut slapi_value
}
Value --> ValueRef
class SlapiMods {
+inner: *const libc::c_void
+vas: Vec<ValueArray>
+new() Self
+append(modtype, attrtype, values)
}
class Modify {
+pb: Pblock
+mods: SlapiMods
+new(dn: &SdnRef, mods: SlapiMods, plugin_id: &PluginIdRef) Result<Self, PluginError>
+execute() Result<ModifyResult, LDAPError>
}
Modify --> SlapiMods
Modify --> Pblock
class PluginIdRef {
+raw_pid: *const libc::c_void
}
Modify --> PluginIdRef
class Search {
+pb: Pblock
+filter: Option<CString>
+stype: SearchType
+new_map_entry(basedn: &SdnRef, scope: SearchScope, filter: &str, plugin_id: &PluginIdRef, cbdata: &T, mapfn) Result<Self, PluginError>
+execute() Result<SearchResult, LDAPError>
}
Search --> Pblock
Search --> PluginIdRef
Search --> SdnRef
class BackendRef {
+raw_be: *const libc::c_void
+new(dn: &SdnRef) Result<Self, ()>
+as_ptr() *const libc::c_void
}
BackendRef --> SdnRef
class BackendRefTxn {
+pb: Pblock
+committed: bool
}
BackendRefTxn --> Pblock
BackendRefTxn --> BackendRef
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/plugins/entryuuid/src/lib.rs:47` </location>
<code_context>
+ let config_sdn: Sdn = Sdn::try_from("cn=config").expect("Invalid static dn");
+ let schema_sdn: Sdn = Sdn::try_from("cn=schema").expect("Invalid static dn");
- if sdn.is_below_suffix(&*config_sdn) || sdn.is_below_suffix(&*schema_sdn) {
+ if sdn.is_below_suffix(&config_sdn) || sdn.is_below_suffix(&schema_sdn) {
// We don't need to assign to these suffixes.
log_error!(
</code_context>
<issue_to_address>
Removing dereference of Sdn is a good simplification.
Please verify that all usages of `Sdn` now match the expected type and do not require dereferencing elsewhere.
Suggested implementation:
```rust
let config_sdn: Sdn = Sdn::try_from("cn=config").expect("Invalid static dn");
let schema_sdn: Sdn = Sdn::try_from("cn=schema").expect("Invalid static dn");
// Ensure all usages of Sdn are passed as references, matching method expectations.
if sdn.is_below_suffix(&config_sdn) || sdn.is_below_suffix(&schema_sdn) {
// We don't need to assign to these suffixes.
log_error!(
ErrorLevel::Plugin,
sdn.to_dn_string()
);
```
If there are other usages of `config_sdn` or `schema_sdn` elsewhere in the file or project, ensure they are also passed as references (`&config_sdn`, `&schema_sdn`) where required by method signatures. Remove any remaining dereferencing (`&*config_sdn`, etc.) to maintain consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| if sdn.is_below_suffix(&*config_sdn) || sdn.is_below_suffix(&*schema_sdn) { |
There was a problem hiding this comment.
suggestion: Removing dereference of Sdn is a good simplification.
Please verify that all usages of Sdn now match the expected type and do not require dereferencing elsewhere.
Suggested implementation:
let config_sdn: Sdn = Sdn::try_from("cn=config").expect("Invalid static dn");
let schema_sdn: Sdn = Sdn::try_from("cn=schema").expect("Invalid static dn");
// Ensure all usages of Sdn are passed as references, matching method expectations.
if sdn.is_below_suffix(&config_sdn) || sdn.is_below_suffix(&schema_sdn) {
// We don't need to assign to these suffixes.
log_error!(
ErrorLevel::Plugin,
sdn.to_dn_string()
);If there are other usages of config_sdn or schema_sdn elsewhere in the file or project, ensure they are also passed as references (&config_sdn, &schema_sdn) where required by method signatures. Remove any remaining dereferencing (&*config_sdn, etc.) to maintain consistency.
|
@vashirov, hello! I checked other PR's and they share the same failing tests, are these okayish to fail? |
No features added or removed, mostly lint changes from clippy
I might add a few modules for
slapi_r_pluginlater on, so as a first step to get familiar with your workflow and contrubiton I opened this PRI used https://www.port389.org/docs/389ds/contributing.html for reference, sorry if i did something wrong
Summary by Sourcery
Apply lint and style improvements across Rust plugins, modernizing FFI declarations, pointer handling, and general Rust idioms while maintaining existing functionality.
Enhancements:
Build:
Tests: