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

Stop triggering no_magic_numbers rule on literals used in ranges #5431

Merged
merged 1 commit into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#1762](https://github.com/realm/SwiftLint/pull/1762)

* Stop triggering `no_magic_numbers` rule on literals used in range
expressions assigned to variables.
[SimplyDanny](https://github.com/SimplyDanny)
[#5430](https://github.com/realm/SwiftLint/pull/5430)

* Add `affect_initializers` option to allow `unneeded_override` rule
to affect initializers.
[leonardosrodrigues0](https://github.com/leonardosrodrigues0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import SwiftSyntax

@SwiftSyntaxRule
@SwiftSyntaxRule(foldExpressions: true)
struct NoMagicNumbersRule: OptInRule {
var configuration = NoMagicNumbersConfiguration()

Expand Down Expand Up @@ -75,18 +75,24 @@ struct NoMagicNumbersRule: OptInRule {
Example("let foo = 2 >> 2"),
Example("let foo = 2 << 2"),
Example("let a = b / 100.0"),
Example("let range = 2 ..< 12"),
Example("let range = ...12"),
Example("let range = 12..."),
Example("let (lowerBound, upperBound) = (400, 599)"),
Example("let a = (5, 10)"),
Example("""
let notFound = (statusCode: 404, description: "Not Found", isError: true)
""")
Example("let notFound = (statusCode: 404, description: \"Not Found\", isError: true)")
],
triggeringExamples: [
Example("foo(↓321)"),
Example("bar(↓1_000.005_01)"),
Example("array[↓42]"),
Example("let box = array[↓12 + ↓14]"),
Example("let a = b + ↓2.0"),
Example("let range = 2 ... ↓12 + 1"),
Example("let range = ↓2*↓6..."),
Example("let slice = array[↓2...↓4]"),
Example("for i in ↓3 ..< ↓8 {}"),
Example("let n: Int = Int(r * ↓255) << ↓16 | Int(g * ↓255) << ↓8"),
Example("Color.primary.opacity(isAnimate ? ↓0.1 : ↓1.5)"),
Example("""
class MyTest: XCTestCase {}
Expand Down Expand Up @@ -143,7 +149,7 @@ private extension NoMagicNumbersRule {
if node.isMemberOfATestClass(configuration.testParentClasses) {
return
}
if node.isOperandOfBitwiseShiftOperation() {
if node.isOperandOfFreestandingShiftOperation() {
return
}
let violation = node.positionAfterSkippingLeadingTrivia
Expand Down Expand Up @@ -181,8 +187,24 @@ private extension TokenSyntax {
guard let grandparent = parent?.parent else {
return true
}
return !grandparent.is(InitializerClauseSyntax.self)
&& grandparent.as(PrefixOperatorExprSyntax.self)?.parent?.is(InitializerClauseSyntax.self) != true
if grandparent.is(InitializerClauseSyntax.self) {
return false
}
let operatorParent = grandparent.as(PrefixOperatorExprSyntax.self)?.parent
?? grandparent.as(PostfixOperatorExprSyntax.self)?.parent
?? grandparent.asAcceptedInfixOperator?.parent
return operatorParent?.is(InitializerClauseSyntax.self) != true
}
}

private extension Syntax {
var asAcceptedInfixOperator: InfixOperatorExprSyntax? {
if let infixOp = `as`(InfixOperatorExprSyntax.self),
let operatorSymbol = infixOp.operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind,
[.binaryOperator("..."), .binaryOperator("..<")].contains(operatorSymbol) {
return infixOp
}
return nil
}
}

Expand Down Expand Up @@ -211,19 +233,12 @@ private extension ExprSyntaxProtocol {
return nil
}

func isOperandOfBitwiseShiftOperation() -> Bool {
guard
let siblings = parent?.as(ExprListSyntax.self)?.children(viewMode: .sourceAccurate),
siblings.count == 3
else {
return false
}

let operatorIndex = siblings.index(after: siblings.startIndex)
if let tokenKind = siblings[operatorIndex].as(BinaryOperatorExprSyntax.self)?.operator.tokenKind {
return tokenKind == .binaryOperator("<<") || tokenKind == .binaryOperator(">>")
func isOperandOfFreestandingShiftOperation() -> Bool {
if let operation = parent?.as(InfixOperatorExprSyntax.self),
let operatorSymbol = operation.operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind,
[.binaryOperator("<<"), .binaryOperator(">>")].contains(operatorSymbol) {
return operation.parent?.isProtocol((any ExprSyntaxProtocol).self) != true
}

return false
}
}
Expand Down