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

mark breaks when fixing and MARK: is found inside a comment block #3841

Closed
2 tasks done
goranche opened this issue Feb 4, 2022 · 6 comments · Fixed by #3844
Closed
2 tasks done

mark breaks when fixing and MARK: is found inside a comment block #3841

goranche opened this issue Feb 4, 2022 · 6 comments · Fixed by #3844

Comments

@goranche
Copy link
Contributor

goranche commented Feb 4, 2022

New Issue Checklist

Describe the bug

The mark rule breaks if MARK: is found anywhere inside a comment block.

The issue lies in that the mark rule takes the location of the comment block as the location of the MARK: and changes the whole range to // MARK:.
Since the length of the ranges that are converted to // MARK: are wrong, this ultimately causes a crash (more info below)

An example that shows this clearly (without a crash):

/*
MARK: 
*/

This will be converted into (notice the missing opening comment tag on line 0):

// MARK: 
*/

If multiple MARK: are found inside a comment block (example at end of description), the ranges found always start at /*, while the lengths will be calculated to the colon for each MARK:. Too long ranges are then replaced and the resulting string can become too short, which results in an NSInvalidArgumentException - Range or index out of bounds exception here.

The individual ranges are calculated in violationRanges(in:matching:), where it's obvious that the location of the found comment token (syntaxTokens[0].offset) is used, and not the location for the actual MARK.

This issue is specifically related to the spaceStartPattern, but other patterns make use of the same logic and might trigger the same issue.

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint --fix
Correcting Swift files in current working directory
Correcting 'Test.swift' (1/1)
2022-02-04 19:38:30.259 swiftlint[92315:2900341] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFString replaceCharactersInRange:withString:]: Range or index out of bounds'
*** First throw call stack:
(
	0   CoreFoundation                      0x00000001bd1301cc __exceptionPreprocess + 240
	1   libobjc.A.dylib                     0x00000001bce817b8 objc_exception_throw + 60
	2   CoreFoundation                      0x00000001bd200488 -[__NSCFString characterAtIndex:].cold.1 + 0
	3   CoreFoundation                      0x00000001bd1790a8 -[__NSCFAttributedString _tryRetain] + 0
	4   CoreFoundation                      0x00000001bd0c212c CFStringCreateFromExternalRepresentation + 0
	5   Foundation                          0x00000001bdfd4bb4 -[NSString stringByReplacingCharactersInRange:withString:] + 80
	6   swiftlint                           0x0000000104b9ba94 $s18SwiftLintFramework8MarkRuleV7correct33_15BF85CCE99C1757DDF2E6E860F9471BLL4file7pattern13replaceString12keepLastCharSayAA10CorrectionVGAA0aB4FileC_S2SSbtF + 464
	7   swiftlint                           0x0000000104b9b6c0 $s18SwiftLintFramework8MarkRuleV7correct4fileSayAA10CorrectionVGAA0aB4FileC_tF + 96
	8   swiftlint                           0x0000000104aa8d54 $s18SwiftLintFramework8MarkRuleVAA011CorrectableE0A2aDP7correct4file5using17compilerArgumentsSayAA10CorrectionVGAA0aB4FileC_AA0E7StorageCSaySSGtFTWTm + 88
	9   swiftlint                           0x0000000104a77520 $s18SwiftLintFramework15CollectedLinterV7correct5usingSayAA10CorrectionVGAA11RuleStorageC_tF + 1116
	10  swiftlint                           0x0000000104d707d4 $s9swiftlint20LintOrAnalyzeCommandV11autocorrect33_50703DD9A63FFC386F0BA0D6C49778EBLLys6ResultOyytAA05SwiftB5ErrorOGAA0bcD7OptionsVFZy0pB9Framework15CollectedLinterVcfU_ + 120
	11  swiftlint                           0x0000000104d65920 $s18SwiftLintFramework13ConfigurationV9swiftlintE5visit33_30E847BB8C9F32DCEAC767137A79F9B7LL7linters7visitor7storage18duplicateFileNamesSayAA0abS0CGSayAA15CollectedLinterVG_AD20LintableFilesVisitorVAA11RuleStorageCShySSGtFAlOcfU_yyXEfU0_ + 424
	12  swiftlint                           0x0000000104d68fbc $ss5Error_pIgzo_ytsAA_pIegrzo_TRTA + 20
	13  swiftlint                           0x0000000104d69238 $ss5Error_pIgzo_ytsAA_pIegrzo_TRTA.17 + 12
	14  libswiftObjectiveC.dylib            0x00000001d188cacc $s10ObjectiveC15autoreleasepool8invokingxxyKXE_tKlF + 64
	15  swiftlint                           0x0000000104d655b0 $s18SwiftLintFramework13ConfigurationV9swiftlintE5visit33_30E847BB8C9F32DCEAC767137A79F9B7LL7linters7visitor7storage18duplicateFileNamesSayAA0abS0CGSayAA15CollectedLinterVG_AD20LintableFilesVisitorVAA11RuleStorageCShySSGtFAlOcfU_ + 712
	16  swiftlint                           0x0000000104d690f0 $s18SwiftLintFramework15CollectedLinterVAA0aB4FileCIggo_AcEIegnr_TRTA + 80
	17  swiftlint                           0x0000000104d61dd8 $sSa9swiftlintE11parallelMap9transformSayqd__Gqd__xXE_tlFySryqd__Gz_SiztXEfU_ySiXEfU_18SwiftLintFramework15CollectedLinterV_AF0eF4FileCTg5 + 360
	18  libswiftDispatch.dylib              0x00000001cf4c1300 $sSiIgy_SiIegy_TRTA + 28
	19  libswiftDispatch.dylib              0x00000001cf4c132c $sSiIegy_SiIyBy_TR + 32
	20  libdispatch.dylib                   0x00000001bce26bec _dispatch_client_callout2 + 20
	21  libdispatch.dylib                   0x00000001bce3b51c _dispatch_apply_serial + 164
	22  libdispatch.dylib                   0x00000001bce26bac _dispatch_client_callout + 20
	23  libdispatch.dylib                   0x00000001bce2bfd0 _dispatch_sync_function_invoke + 52
	24  libdispatch.dylib                   0x00000001bce3ad88 _dispatch_apply_with_attr_f + 1324
	25  libswiftDispatch.dylib              0x00000001cf4c13d0 _swift_dispatch_apply_current + 128
	26  libswiftDispatch.dylib              0x00000001cf4c1284 $sSo17OS_dispatch_queueC8DispatchE17concurrentPerform10iterations7executeySi_ySiXEtFZ + 196
	27  swiftlint                           0x0000000104d6811c $sSa28_unsafeUninitializedCapacity16initializingWithSayxGSi_ySryxGz_SiztKXEtKcfC18SwiftLintFramework0fG4FileC_Tg5079$sSa9swiftlintE11parallelMap9transformSayqd__Gqd__xXE_tlFySryqd__Gz_SiztXEfU_18fg34Framework15CollectedLinterV_AF0eF4I4CTg5SayAE0xY0VGxq_r0_lyAjGIsgnr_Tf1ncn_nTf4nngn_n + 192
	28  swiftlint                           0x0000000104d6855c $s18SwiftLintFramework13ConfigurationV9swiftlintE5visit33_30E847BB8C9F32DCEAC767137A79F9B7LL7linters7visitor7storage18duplicateFileNamesSayAA0abS0CGSayAA15CollectedLinterVG_AD20LintableFilesVisitorVAA11RuleStorageCShySSGtFTf4nndnn_n + 1024
	29  swiftlint                           0x0000000104d62e1c $s18SwiftLintFramework13ConfigurationV9swiftlintE18visitLintableFiles4with7storages6ResultOySayAA0aB4FileCGAD0aB5ErrorOGAD0gH7VisitorV_AA11RuleStorageCtF + 2116
	30  swiftlint                           0x0000000104d720c4 $s9swiftlint20LintOrAnalyzeCommandV11autocorrect33_50703DD9A63FFC386F0BA0D6C49778EBLLys6ResultOyytAA05SwiftB5ErrorOGAA0bcD7OptionsVFZTf4nd_n + 512
	31  swiftlint                           0x0000000104d6f600 $s9swiftlint20LintOrAnalyzeCommandV3runys6ResultOyytAA05SwiftB5ErrorOGAA0bcD7OptionsVFZ + 692
	32  swiftlint                           0x0000000104d59668 $s9swiftlint9SwiftLintV0C0V3runyyKF + 6136
	33  swiftlint                           0x0000000104d59a70 $s9swiftlint9SwiftLintV0C0V14ArgumentParser15ParsableCommandAafGP3runyyKFTW + 12
	34  swiftlint                           0x0000000104978b9c $s14ArgumentParser15ParsableCommandPAAE4mainyySaySSGSgFZ + 92
	35  swiftlint                           0x0000000104d60cc4 $s9swiftlint9SwiftLintV30mainHandlingDeprecatedCommandsyySaySSGSgFZTf4nd_n + 352
	36  swiftlint                           0x0000000104d51904 $s9swiftlintyycfU_ + 16
	37  swiftlint                           0x0000000104a5c2bc $sIeg_IeyB_TR + 20
	38  libdispatch.dylib                   0x00000001bce24e60 _dispatch_call_block_and_release + 32
	39  libdispatch.dylib                   0x00000001bce26bac _dispatch_client_callout + 20
	40  libdispatch.dylib                   0x00000001bce29cd4 _dispatch_queue_override_invoke + 792
	41  libdispatch.dylib                   0x00000001bce3831c _dispatch_root_queue_drain + 396
	42  libdispatch.dylib                   0x00000001bce38b58 _dispatch_worker_thread2 + 164
	43  libsystem_pthread.dylib             0x00000001bcfe12c8 _pthread_wqthread + 228
	44  libsystem_pthread.dylib             0x00000001bcfe0018 start_wqthread + 8
)
libc++abi: terminating with uncaught exception of type NSException
[1]    92315 abort      swiftlint --fix

Environment

  • SwiftLint version: 0.46.2

  • Installation method used: Homebrew and built from sources (for debugging)

  • No configuration file was used

  • Xcode 13.2.1 - Build version 13C100

  • Sample that breaks on swiftlint --fix:

import Foundation
/*
MARK: 1
 // MARK: 2
// MARK: 3
*/
@goranche
Copy link
Contributor Author

goranche commented Feb 4, 2022

An example from real life is a simple commented out block of code that contained MARKS:

import Foundation

/*
let release = "release/1.3.0"
let base = "https://raw.githubusercontent.com/ehn-dcc-development/ehn-dcc-schema/\(release)/valuesets"

guard let baseURL = URL(string: base) else {
	fatalError("Couldn't convert base baseUrl to a URL")
}

let unknownCase = "\tcase unknown = \"unknown\""
let unknownDisplay = "\t\tcase .unknown: return \"unknown\""
let displaySequence = [
	"\tpublic var display: String {",
	"\t\tswitch self {",
	unknownDisplay
]
let closingSequence = [
	"\t\t}",
	"\t}",
	"}",
	""
]

// MARK: - ValueConverter

protocol ValueConverter {
	var input: String { get }
	var output: String { get }
	func output(_ values: [String: EUValSetValue], to filename: String)
}

// MARK: - CountryCode
*/

@goranche
Copy link
Contributor Author

goranche commented Feb 4, 2022

A possible solution would be to change the patterns that are being searched for and not rely on the fact that a // MARK: is always a separate comment token 🤷‍♂️

@SimplyDanny
Copy link
Collaborator

Duplicate of #1749?

@goranche
Copy link
Contributor Author

goranche commented Feb 5, 2022

Duplicate of #1749?

😱 yes, yes it is... 🙈
I'm sorry, I promise I tried searching existing issues, but ... yeah 😊

should I just close this issue (and possibly add the information to the other issue)?

edit: actually, I'll fix it and create a PR 🙈

goranche added a commit to goranche/SwiftLint that referenced this issue Feb 5, 2022
@SimplyDanny
Copy link
Collaborator

If your PR fixes the bug both issues will be closed anyway. Otherwise

should I just close this issue (and possibly add the information to the other issue)?

would possibly be preferred.

@goranche
Copy link
Contributor Author

goranche commented Feb 5, 2022

If your PR fixes the bug both issues will be closed anyway.

that was my reasoning as well, so I left it here, while mentioning both issues in the PR (with a "fixes" tag) 🙈

jpsim pushed a commit to goranche/SwiftLint that referenced this issue Mar 4, 2022
jpsim pushed a commit to goranche/SwiftLint that referenced this issue Mar 4, 2022
jpsim pushed a commit that referenced this issue Mar 4, 2022
coffmark pushed a commit to coffmark/SwiftLint that referenced this issue Apr 11, 2022
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 a pull request may close this issue.

2 participants