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

Add CGGeometry legacy functions rule #618

Merged
merged 10 commits into from
Apr 18, 2016

Conversation

blaisesarr
Copy link
Contributor

@blaisesarr blaisesarr commented Apr 15, 2016

This PR add new LegacyCGGeometryFunctionsRule rule

This rule enforce the use of new Swift CGGeometry methods and properties(like rect.width) instead of legacy CGGeometry functions(like CGRectGetWidth(rect))

This closes #625

@blaisesarr blaisesarr changed the title Add cggeometry legacy functions rule Add CGGeometry legacy functions rule Apr 15, 2016
@jpsim
Copy link
Collaborator

jpsim commented Apr 15, 2016

Looks good to me, thoughts @scottrhoyt?

@jpsim
Copy link
Collaborator

jpsim commented Apr 15, 2016

Could you please add a changelog entry for this @bsarr007?

@scottrhoyt
Copy link
Contributor

Looks great to me!

@jpsim
Copy link
Collaborator

jpsim commented Apr 15, 2016

Great! Just needs a changelog entry now.

let regularExpression = regex(pattern)
for range in matches.reverse() {
contents = regularExpression.stringByReplacingMatchesInString(contents,
options: [], range: range, withTemplate: template)
Copy link
Collaborator

@norio-nomura norio-nomura Apr 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This range is byte range. We need converting byte range to utf16 based range by using NSString.byteRangeToNSRange(start:length:) that offered by SourceKitten.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I found same mistakes are used in correctFile(file:) on another rules…

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That cause crash by running swiftlint autocorrect --path test/618.swift
test/618.swift:

CGRectGetWidth(rect)
CGRectGetHeight(rect)
CGRectGetMinX(rect)
CGRectGetMidX(rect)
CGRectGetMaxX(rect)
CGRectGetMinY(rect)
CGRectGetMidY(rect)
CGRectGetMaxY(rect)
CGRectIsNull(rect)
CGRectIsEmpty(rect)
CGRectIsInfinite(rect)
CGRectStandardize(rect)
CGRectIntegral(rect)
CGRectInset(rect, 10, 5)
CGRectOffset(rect, -2, 8.3)
CGRectUnion(rect1, rect2)
CGRectIntersection(rect1, rect2)
CGRectContainsRect(rect1, rect2)
CGRectContainsPoint(rect, point)
CGRectIntersectsRect(rect1, rect2)
$ swiftlint autocorrect --config ~/.swiftlint-test.yml --path test/618.swift
Loading configuration from '/Users/norio/.swiftlint-test.yml'
Correcting Swift files at path test/618.swift
Correcting '618.swift' (1/1)
2016-04-17 09:31:19.104 swiftlint[24997:3063778] *** Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[NSRegularExpression enumerateMatchesInString:options:range:usingBlock:]: Range or index out of bounds'
*** First throw call stack:
(
    0   CoreFoundation                      0x00007fff9136d4f2 __exceptionPreprocess + 178
    1   libobjc.A.dylib                     0x00007fff8b4fef7e objc_exception_throw + 48
    2   CoreFoundation                      0x00007fff913d44bd +[NSException raise:format:] + 205
    3   Foundation                          0x00007fff99e55f05 -[NSRegularExpression(NSMatching) enumerateMatchesInString:options:range:usingBlock:] + 347
    4   Foundation                          0x00007fff99e55d98 -[NSRegularExpression(NSMatching) matchesInString:options:range:] + 159
    5   Foundation                          0x00007fff99eed9ec -[NSRegularExpression(NSReplacement) stringByReplacingMatchesInString:options:range:withTemplate:] + 292
    6   SwiftLintFramework                  0x000000010d9d305d _TTSf4g_d___TFV18SwiftLintFramework29LegacyCGGeometryFunctionsRule11correctFilefC21SourceKittenFramework4FileGSaVS_10Correction_ + 16365
    7   SwiftLintFramework                  0x000000010d9c607f _TTWV18SwiftLintFramework29LegacyCGGeometryFunctionsRuleS_15CorrectableRuleS_FS1_11correctFilefC21SourceKittenFramework4FileGSaVS_10Correction_ + 15
    8   SwiftLintFramework                  0x000000010d9f85d6 _TFV18SwiftLintFramework6Linter7correctfT_GSaVS_10Correction_ + 502
    9   swiftlint                           0x000000010d9af4b8 swiftlint + 132280
    10  swiftlint                           0x000000010d9aed56 swiftlint + 130390
    11  swiftlint                           0x000000010d997950 swiftlint + 35152
    12  swiftlint                           0x000000010d9af10a swiftlint + 131338
    13  swiftlint                           0x000000010d9aee3f swiftlint + 130623
    14  swiftlint                           0x000000010d9af1be swiftlint + 131518
    15  Result                              0x000000010db8c94d _TFO6Result6Result8analysisurfT9ifSuccessFxqd__9ifFailureFq_qd___qd__ + 381
    16  Result                              0x000000010db8ddbb _TTWu0_R_s9ErrorTyperGO6Result6Resultxq__S0_10ResultTypeS0_FS2_8analysisurfT9ifSuccessFwx5Valueqd__9ifFailureFwx5Errorqd___qd__ + 235
    17  Result                              0x000000010db8afd1 _TFE6ResultPS_10ResultType7flatMapurfFwx5ValueGOS_6Resultqd__wx5Error_GS2_qd__wxS3__ + 145
    18  swiftlint                           0x000000010d9aea1a swiftlint + 129562
    19  swiftlint                           0x000000010d993ebd swiftlint + 20157
    20  Commandant                          0x000000010db53e03 _TFFV10Commandant14CommandWrappercuRd__S_11CommandTypexzWd__7Options11ClientError_wd__11ClientErrorzWd__S2_S3__rFqd__GS0_x_U_FCS_14ArgumentParserGO6Result6ResultT_GOS_15CommandantErrorQd____ + 2051
    21  Commandant                          0x000000010db532a4 _TPA__TFFV10Commandant14CommandWrappercuRd__S_11CommandTypexzWd__7Options11ClientError_wd__11ClientErrorzWd__S2_S3__rFqd__GS0_x_U_FCS_14ArgumentParserGO6Result6ResultT_GOS_15CommandantErrorQd____ + 132
    22  Commandant                          0x000000010db5194e _TFC10Commandant15CommandRegistry10runCommandfTSS9argumentsGSaSS__GSqGO6Result6ResultT_GOS_15CommandantErrorx___ + 318
    23  Commandant                          0x000000010db51e7a _TFC10Commandant15CommandRegistry4mainfT9argumentsGSaSS_11defaultVerbSS12errorHandlerFxT__T_ + 538
    24  Commandant                          0x000000010db51c57 _TFC10Commandant15CommandRegistry4mainfT11defaultVerbSS12errorHandlerFxT__T_ + 135
    25  swiftlint                           0x000000010d994edc swiftlint + 24284
    26  libdispatch.dylib                   0x00007fff97be993d _dispatch_call_block_and_release + 12
    27  libdispatch.dylib                   0x00007fff97bde40b _dispatch_client_callout + 8
    28  libdispatch.dylib                   0x00007fff97be229b _dispatch_root_queue_drain + 1890
    29  libdispatch.dylib                   0x00007fff97be1b00 _dispatch_worker_thread3 + 91
    30  libsystem_pthread.dylib             0x00007fff8b8544de _pthread_wqthread + 1129
    31  libsystem_pthread.dylib             0x00007fff8b852341 start_wqthread + 13
)
libc++abi.dylib: terminating with uncaught exception of type NSException
[1]    24997 abort      swiftlint autocorrect --config ~/.swiftlint-test.yml --path test/618.swift

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #623.
I'll fix existing rules.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This range is byte range. We need converting byte range to utf16 based range by using NSString.byteRangeToNSRange(start:length:) that offered by SourceKitten.

Oops! sorry that's incorrect. The issue is caused by:

       for (pattern, template) in patterns {
           let matches = file.matchPattern(pattern)

We need to search in contents instead of file.

Copy link
Collaborator

@norio-nomura norio-nomura Apr 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed #624.
Could you please refer the difference of LegacyConstantRule.swift? https://github.com/realm/SwiftLint/pull/624/files#diff-c5efff4c85dc1d7666ce1fd906c4bc81

Copy link
Contributor Author

@blaisesarr blaisesarr Apr 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @scottrhoyt @jpsim , thanks for your feedbacks, i added a changelog entry in 7962ac6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @norio-nomura , Good catch! I'll refer to your diff from #624, but why is there a difference between your corrections in LegacyConstantRule.swift and LegacyConstructorRule.swift correctFile: functions?

Copy link
Collaborator

@norio-nomura norio-nomura Apr 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsarr007 Sorry, I should ask you to refer LegacyConstructorRule.swift instead of LegacyConstantRule.swift.

Maybe you mean the following difference.
LegacyConstantRule.swift:

                let matches = file.matchPattern(pattern, withSyntaxKinds: [.Identifier])

LegacyConstructorRule.swift

                let matches = file.matchPattern(pattern)
                    .filter { $0.1.first == .Identifier }

That comes from matching pattern.
The match range of LegacyConstantRule's pattern contains one SyntaxKind that is .Identifier.
The match range of LegacyConstructorRule's pattern contains some SyntaxKind that starting with a .Identifier and following by another.

It seems you rightly referred LegacyConstructorRule. 👍
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@norio-nomura yes indeed, Thanks!

@blaisesarr blaisesarr force-pushed the add-cggeometry-legacy-functions-rule branch 2 times, most recently from 12cb8bc to 264bc28 Compare April 17, 2016 13:05
@blaisesarr blaisesarr force-pushed the add-cggeometry-legacy-functions-rule branch from 264bc28 to 7962ac6 Compare April 17, 2016 13:20
@blaisesarr blaisesarr force-pushed the add-cggeometry-legacy-functions-rule branch from 0dec53d to 4f5f3a6 Compare April 17, 2016 19:00
@norio-nomura
Copy link
Collaborator

Looks good to me. 👍

@jpsim
Copy link
Collaborator

jpsim commented Apr 18, 2016

Great contribution! Thanks!

@jpsim jpsim merged commit 52daadc into realm:master Apr 18, 2016
@blaisesarr
Copy link
Contributor Author

@jpsim Thanks! it was an honor for me to contribute to this repo!

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

Successfully merging this pull request may close these issues.

Add CGGeometry legacy functions rule
4 participants