From 66d50a87bbc39ebf3190158c54b23befc1716f1d Mon Sep 17 00:00:00 2001 From: PeterKaminski09 Date: Thu, 23 Jan 2020 21:02:56 -0800 Subject: [PATCH] Fixes an issue with string comparison in ChartData for finding a dataset by its label (Fixes #4274) ChartData::getDataSetByLabel has a bug where the string comparison is checking the same string against itself due to the functions parameter having the same name as a variable within the closures scope. This change renames the variable to fix the issue. --- ChartDataTests.swift | 69 +++++++++++++++++++ Charts.xcodeproj/project.pbxproj | 4 ++ .../Implementations/Standard/ChartData.swift | 6 +- 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 ChartDataTests.swift diff --git a/ChartDataTests.swift b/ChartDataTests.swift new file mode 100644 index 0000000000..4e4cdbab94 --- /dev/null +++ b/ChartDataTests.swift @@ -0,0 +1,69 @@ +// +// ChartDataTests.swift +// ChartsTests +// +// Created by Peter Kaminski on 1/23/20. +// + +import XCTest +@testable import Charts + +class ChartDataTests: XCTestCase { + + var data: ScatterChartData! + + private enum SetLabels { + static let one = "label1" + static let two = "label2" + static let three = "label3" + static let badLabel = "Bad label" + } + + override func setUp() { + super.setUp() + + let setCount = 5 + let range: UInt32 = 32 + let values1 = (0.. ChartDataEntry in + let val = Double(arc4random_uniform(range) + 3) + return ChartDataEntry(x: Double(i), y: val) + } + let values2 = (0.. ChartDataEntry in + let val = Double(arc4random_uniform(range) + 3) + return ChartDataEntry(x: Double(i), y: val) + } + let values3 = (0.. ChartDataEntry in + let val = Double(arc4random_uniform(range) + 3) + return ChartDataEntry(x: Double(i), y: val) + } + + let set1 = ScatterChartDataSet(entries: values1, label: SetLabels.one) + let set2 = ScatterChartDataSet(entries: values2, label: SetLabels.two) + let set3 = ScatterChartDataSet(entries: values3, label: SetLabels.three) + + data = ScatterChartData(dataSets: [set1, set2, set3]) + } + + func testGetDataSetByLabelCaseSensitive() { + XCTAssertTrue(data.getDataSetByLabel(SetLabels.one, ignorecase: false)?.label == SetLabels.one) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.two, ignorecase: false)?.label == SetLabels.two) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.three, ignorecase: false)?.label == SetLabels.three) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.one.uppercased(), ignorecase: false) == nil) + } + + func testGetDataSetByLabelIgnoreCase() { + XCTAssertTrue(data.getDataSetByLabel(SetLabels.one, ignorecase: true)?.label == SetLabels.one) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.two, ignorecase: true)?.label == SetLabels.two) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.three, ignorecase: true)?.label == SetLabels.three) + + XCTAssertTrue(data.getDataSetByLabel(SetLabels.one.uppercased(), ignorecase: true)?.label == SetLabels.one) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.two.uppercased(), ignorecase: true)?.label == SetLabels.two) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.three.uppercased(), ignorecase: true)?.label == SetLabels.three) + } + + func testGetDataSetByLabelNilWithBadLabel() { + XCTAssertTrue(data.getDataSetByLabel(SetLabels.badLabel, ignorecase: true) == nil) + XCTAssertTrue(data.getDataSetByLabel(SetLabels.badLabel, ignorecase: false) == nil) + } +} + diff --git a/Charts.xcodeproj/project.pbxproj b/Charts.xcodeproj/project.pbxproj index 3db23a5772..fbc4e7478d 100644 --- a/Charts.xcodeproj/project.pbxproj +++ b/Charts.xcodeproj/project.pbxproj @@ -117,6 +117,7 @@ B6DCC229615EFE706F64A37D /* LineScatterCandleRadarRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 923206233CA89FD03565FF87 /* LineScatterCandleRadarRenderer.swift */; }; B85DEB06B4C1AFFC8A0E3295 /* CircleShapeRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = ECE1B1623D3AF69CECAE8562 /* CircleShapeRenderer.swift */; }; BEFD9518F3A74ACF8FA33308 /* Charts.h in Headers */ = {isa = PBXBuildFile; fileRef = 4F9922F0641F7955DC6CD324 /* Charts.h */; settings = {ATTRIBUTES = (Public, ); }; }; + C03E6D8123DAAB2600083010 /* ChartDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C03E6D8023DAAB2600083010 /* ChartDataTests.swift */; }; C04D269AD4A373FD2B621C43 /* LineChartData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C978F31F23C7D21197DC2A1 /* LineChartData.swift */; }; C09E91F67A4AC43C277E7D82 /* BubbleChartDataEntry.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD8ED233775EEC31243A6919 /* BubbleChartDataEntry.swift */; }; C20A62D8CB9120523D5FB650 /* LegendEntry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7C673B9ED4340F550A9283 /* LegendEntry.swift */; }; @@ -286,6 +287,7 @@ BD02157CF8CEE1189BF681DA /* PieChartDataEntry.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PieChartDataEntry.swift; path = Source/Charts/Data/Implementations/Standard/PieChartDataEntry.swift; sourceTree = ""; }; BD5C6D20243EC2F19069AACD /* CandleStickChartRenderer.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = CandleStickChartRenderer.swift; path = Source/Charts/Renderers/CandleStickChartRenderer.swift; sourceTree = ""; }; BFABD027DAF6851088F002AC /* LineChartDataProvider.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = LineChartDataProvider.swift; path = Source/Charts/Interfaces/LineChartDataProvider.swift; sourceTree = ""; }; + C03E6D8023DAAB2600083010 /* ChartDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChartDataTests.swift; sourceTree = ""; }; C31AA65EA27776F8C653C7E8 /* BarChartDataSet.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = BarChartDataSet.swift; path = Source/Charts/Data/Implementations/Standard/BarChartDataSet.swift; sourceTree = ""; }; C52E8344160B5E689DA3C25C /* ChevronDownShapeRenderer.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = ChevronDownShapeRenderer.swift; path = Source/Charts/Renderers/Scatter/ChevronDownShapeRenderer.swift; sourceTree = ""; }; C574E1BC7E12D937A5471EF8 /* Info.plist */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.plist.xml; name = Info.plist; path = "Tests/Supporting Files/Info.plist"; sourceTree = ""; }; @@ -537,6 +539,7 @@ D2E1819D72CD7B6C4A4E8048 /* LineChartTests.swift */, 135F11CD20425AF600D655A3 /* PieChartTests.swift */, 064989451F5C99C7006E8BB3 /* Snapshot.swift */, + C03E6D8023DAAB2600083010 /* ChartDataTests.swift */, ); name = Charts; sourceTree = ""; @@ -985,6 +988,7 @@ B66817462241E3CC00017CF1 /* HorizontalBarChartTests.swift in Sources */, 135F11CE20425AF600D655A3 /* PieChartTests.swift in Sources */, 064989461F5C99C7006E8BB3 /* Snapshot.swift in Sources */, + C03E6D8123DAAB2600083010 /* ChartDataTests.swift in Sources */, 224EFF991FBAAC4700CF9B3B /* EquatableTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/Source/Charts/Data/Implementations/Standard/ChartData.swift b/Source/Charts/Data/Implementations/Standard/ChartData.swift index 1cb1fd93f7..f4699ef702 100644 --- a/Source/Charts/Data/Implementations/Standard/ChartData.swift +++ b/Source/Charts/Data/Implementations/Standard/ChartData.swift @@ -349,10 +349,8 @@ open class ChartData: NSObject // TODO: Return nil instead of -1 if ignorecase { - return dataSets.firstIndex { - guard let label = $0.label else { return false } - return label.caseInsensitiveCompare(label) == .orderedSame - } ?? -1 + return dataSets.firstIndex { $0.label?.caseInsensitiveCompare(label) == .orderedSame } + ?? -1 } else {