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

Review for CA1822: Mark members as static #663

Closed
NightOwl888 opened this issue Oct 15, 2022 · 4 comments
Closed

Review for CA1822: Mark members as static #663

NightOwl888 opened this issue Oct 15, 2022 · 4 comments
Assignees

Comments

@NightOwl888
Copy link
Contributor

NightOwl888 commented Oct 15, 2022

https://sonarcloud.io/project/issues?resolved=false&types=CODE_SMELL&id=nikcio_lucenenet&open=AYPAuPfOhbfJOGLOobHM
https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1822

Members that do not access instance data or call instance methods can be marked static. This may diverge from the Java implementation, so we should mark these members with the comment // LUCENENET: CA1822: Mark members as static.

A check should be made against the Lucene source code first before applying this change. If there is no final modifier on a method in Java...

public void DoSomething() {
   // impl
}

...it means it should be declared virtual in .NET (unless the class is sealed).

public virtual void DoSomething()
{
   // impl
}

In this case, we cannot declare the method/property static and instead add the missing virtual modifier.

`NOTE: The entire codebase has been manually reviewed already to catch the virtual/non-virtual declarations, however, since this review was manual, there may have been some that were missed.

@NightOwl888 NightOwl888 added performance up-for-grabs This issue is open to be worked on by anyone good-first-issue Good for newcomers is:enhancement New feature or request hacktoberfest-accepted labels Oct 15, 2022
@nikcio
Copy link
Contributor

nikcio commented Oct 15, 2022

@nikcio
Copy link
Contributor

nikcio commented Oct 16, 2022

I'll look into this one

@nikcio
Copy link
Contributor

nikcio commented Oct 26, 2022

No more issues have been detected. This issue is ready to be closed.

@NightOwl888
Copy link
Contributor Author

Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants