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

Show warning if // periphery:ignore unneeded #427

Open
subdan opened this issue Nov 18, 2021 · 7 comments
Open

Show warning if // periphery:ignore unneeded #427

subdan opened this issue Nov 18, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@subdan
Copy link

subdan commented Nov 18, 2021

I want to see a warning if // periphery:ignore command is unneeded.

// periphery:ignore
func sayHello() {
    print("sayHello")
}
sayHello()

I want to see the following warning:
Superfluous Ignore Command Violation: function sayHello is used and shouldn't marked as ignored. Please remove the ignore command.


SwiftLint shows a warning if a disable command doesn't needed.

// swiftlint:disable:next function_body_length
func sayHello() {
    print("Hello")
}

Warning:
Superfluous Disable Command Violation: SwiftLint rule 'function_body_length' did not trigger a violation in the disabled region. Please remove the disable command. (superfluous_disable_command)

@ileitch ileitch added the enhancement New feature or request label Nov 18, 2021
@ileitch ileitch changed the title [Feature Request] Show warning if // periphery:ignore unneeded Show warning if // periphery:ignore unneeded Jan 15, 2023
@maxwellE
Copy link

maxwellE commented Jun 2, 2023

@subdan Are you working on this? I would be interested in adding this capability

@ileitch
Copy link
Contributor

ileitch commented Jun 2, 2023

@maxwellE @geraldWilliam took a stab at this recently: #551. It wasn't quite the right solution though. I've been wanting to take another look but haven't had time yet. I'll be focusing more on integrating Periphery into Reddit's build system soon, so hopefully I can allocate some time for this too.

@subdan
Copy link
Author

subdan commented Jun 3, 2023

@subdan Are you working on this? I would be interested in adding this capability

No

@maxwellE
Copy link

@ileitch was there any progress on this yet?

@tinder-maxwellelliott
Copy link

@ileitch Id like to take this up if you have any pointers on how to best tackle this

@ileitch
Copy link
Contributor

ileitch commented Sep 8, 2023

Hey @maxwellE, sorry for the slow reply. I haven't had time to think about the right solution for this. I think a naive approach is possibly quite simple, but finding one that doesn't significantly impact performance might be a bit harder. Please have a go!

@maxwellE
Copy link

Would it make sense to have even a less performant version of this implemented behind a flag? I could dive into how SwiftLint accomplishes this same logic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants