Skip to content

Commit

Permalink
GH-41602: [C#] Resolve build warnings (#41645)
Browse files Browse the repository at this point in the history 
### What changes are included in this PR?

Adds annotations or suppressions to disable build warnings. Configures projects to produce an error on warnings.

### Are these changes tested?

Changes are covered by existing tests.

Closes #41602

* GitHub Issue: #41602

Authored-by: Curt Hagenlocher <curt@hagenlocher.org>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
  • Loading branch information
@CurtHagenlocher
CurtHagenlocher committed May 14, 2024
1 parent bd89c42 commit e411e0e
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 40 deletions.
4 changes: 3 additions & 1 deletion csharp/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,13 @@
<LangVersion>latest</LangVersion>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>$(CSharpDir)ApacheArrow.snk</AssemblyOriginatorKeyFile>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<!-- NuGet properties -->
<PropertyGroup>
<Authors>The Apache Software Foundation</Authors>
<PackageIconUrl>https://www.apache.org/images/feather.png</PackageIconUrl>
<PackageIcon>feather.png</PackageIcon>
<!-- We can't use PackageLicenseExpression; the license file also contains 3rd-party notices. -->
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
<PackageProjectUrl>https://arrow.apache.org/</PackageProjectUrl>
Expand All @@ -55,6 +56,7 @@

<ItemGroup Condition="'$(IsPackable)' == 'true'">
<Content Include="$(RepoRoot)LICENSE.txt" Pack="true" PackagePath="" />
<Content Include="$(CSharpDir)/feather.png" Link="feather.png" Pack="true" PackagePath="\" />
</ItemGroup>

</Project>
Binary file added csharp/feather.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions csharp/src/Apache.Arrow/Arrays/BinaryArray.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ public ReadOnlySpan<byte> GetBytes(int index, out bool isNull)

int ICollection<byte[]>.Count => Length;
bool ICollection<byte[]>.IsReadOnly => true;
void ICollection<byte[]>.Add(byte[]? item) => throw new NotSupportedException("Collection is read-only.");
bool ICollection<byte[]>.Remove(byte[]? item) => throw new NotSupportedException("Collection is read-only.");
void ICollection<byte[]>.Add(byte[] item) => throw new NotSupportedException("Collection is read-only.");
bool ICollection<byte[]>.Remove(byte[] item) => throw new NotSupportedException("Collection is read-only.");
void ICollection<byte[]>.Clear() => throw new NotSupportedException("Collection is read-only.");

bool ICollection<byte[]>.Contains(byte[] item)
Expand Down
14 changes: 8 additions & 6 deletions csharp/src/Apache.Arrow/Arrays/Decimal256Array.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#nullable enable

using System;
using System.Collections;
using System.Collections.Generic;
Expand All @@ -23,7 +25,7 @@

namespace Apache.Arrow
{
public class Decimal256Array : FixedSizeBinaryArray, IReadOnlyList<SqlDecimal?>, IReadOnlyList<string>
public class Decimal256Array : FixedSizeBinaryArray, IReadOnlyList<SqlDecimal?>, IReadOnlyList<string?>
{
public class Builder : BuilderBase<Decimal256Array, Builder>
{
Expand Down Expand Up @@ -178,7 +180,7 @@ public Decimal256Array(ArrayData data)
return list;
}

public string GetString(int index)
public string? GetString(int index)
{
if (IsNull(index))
{
Expand Down Expand Up @@ -230,17 +232,17 @@ public bool TryGetSqlDecimal(int index, out SqlDecimal? value)
}
}

int IReadOnlyCollection<string>.Count => Length;
string? IReadOnlyList<string>.this[int index] => GetString(index);
int IReadOnlyCollection<string?>.Count => Length;
string? IReadOnlyList<string?>.this[int index] => GetString(index);

IEnumerator<string> IEnumerable<string>.GetEnumerator()
IEnumerator<string?> IEnumerable<string?>.GetEnumerator()
{
for (int index = 0; index < Length; index++)
{
yield return GetString(index);
}
}

IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable<string>)this).GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable<string?>)this).GetEnumerator();
}
}
2 changes: 2 additions & 0 deletions csharp/src/Apache.Arrow/Memory/NativeMemoryManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ internal NativeMemoryManager(INativeAllocationOwner owner, IntPtr ptr, int offse
_owner = owner;
}

#pragma warning disable CA2015 // TODO: is this correct?
~NativeMemoryManager()
{
Dispose(false);
}
#pragma warning restore CA2015

public override unsafe Span<byte> GetSpan()
{
Expand Down
25 changes: 13 additions & 12 deletions csharp/test/Apache.Arrow.Flight.Sql.Tests/FlightSqlServerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.

#nullable enable

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
Expand Down Expand Up @@ -65,7 +66,7 @@ public async Task EnsureTheCorrectActionsAreGiven()
var streamWriter = new MockServerStreamWriter<FlightActionType>();

//When
await producer.ListActions(streamWriter, new MockServerCallContext()).ConfigureAwait(false);
await producer.ListActions(streamWriter, new MockServerCallContext());
var actions = streamWriter.Messages.ToArray();

Assert.Equal(FlightSqlUtils.FlightSqlActions, actions);
Expand Down Expand Up @@ -115,7 +116,7 @@ public void EnsureTableSchemaIsCorrectWithoutTableSchema(bool includeTableSchema
[InlineData(typeof(CommandGetImportedKeys), "GetImportedKeysFlightInfo")]
[InlineData(typeof(CommandGetCrossReference), "GetCrossReferenceFlightInfo")]
[InlineData(typeof(CommandGetXdbcTypeInfo), "GetXdbcTypeFlightInfo")]
public async void EnsureGetFlightInfoIsCorrectlyRoutedForCommand(Type commandType, string expectedResult)
public async Task EnsureGetFlightInfoIsCorrectlyRoutedForCommand(Type commandType, string expectedResult)
{
//Given
var command = (IMessage) Activator.CreateInstance(commandType)!;
Expand All @@ -131,7 +132,7 @@ public async void EnsureGetFlightInfoIsCorrectlyRoutedForCommand(Type commandTyp


[Fact]
public async void EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupportedAndHasNoDescriptor()
public async Task EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupportedAndHasNoDescriptor()
{
//Given
var producer = new TestFlightSqlSever();
Expand All @@ -145,7 +146,7 @@ public async void EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupp
}

[Fact]
public async void EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupported()
public async Task EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupported()
{
//Given
var producer = new TestFlightSqlSever();
Expand Down Expand Up @@ -175,7 +176,7 @@ public async void EnsureAnInvalidOperationExceptionIsThrownWhenACommandIsNotSupp
[InlineData(typeof(CommandGetImportedKeys), "DoGetImportedKeys")]
[InlineData(typeof(CommandGetCrossReference), "DoGetCrossReference")]
[InlineData(typeof(CommandGetXdbcTypeInfo), "DoGetXbdcTypeInfo")]
public async void EnsureDoGetIsCorrectlyRoutedForADoGetCommand(Type commandType, string expectedResult)
public async Task EnsureDoGetIsCorrectlyRoutedForADoGetCommand(Type commandType, string expectedResult)
{
//Given
var producer = new TestFlightSqlSever();
Expand All @@ -192,7 +193,7 @@ public async void EnsureDoGetIsCorrectlyRoutedForADoGetCommand(Type commandType,
}

[Fact]
public async void EnsureAnInvalidOperationExceptionIsThrownWhenADoGetCommandIsNotSupported()
public async Task EnsureAnInvalidOperationExceptionIsThrownWhenADoGetCommandIsNotSupported()
{
//Given
var producer = new TestFlightSqlSever();
Expand All @@ -213,7 +214,7 @@ public async void EnsureAnInvalidOperationExceptionIsThrownWhenADoGetCommandIsNo
[InlineData(SqlAction.CloseRequest, typeof(ActionClosePreparedStatementRequest), "ClosePreparedStatement")]
[InlineData(SqlAction.CreateRequest, typeof(ActionCreatePreparedStatementRequest), "CreatePreparedStatement")]
[InlineData("BadCommand", typeof(ActionCreatePreparedStatementRequest), "Action type BadCommand not supported", true)]
public async void EnsureDoActionIsCorrectlyRoutedForAnActionRequest(string actionType, Type actionBodyType, string expectedResponse, bool isException = false)
public async Task EnsureDoActionIsCorrectlyRoutedForAnActionRequest(string actionType, Type actionBodyType, string expectedResponse, bool isException = false)
{
//Given
var producer = new TestFlightSqlSever();
Expand All @@ -237,19 +238,19 @@ public async void EnsureDoActionIsCorrectlyRoutedForAnActionRequest(string actio
[InlineData(typeof(CommandPreparedStatementQuery), "PutPreparedStatementQuery")]
[InlineData(typeof(CommandPreparedStatementUpdate), "PutPreparedStatementUpdate")]
[InlineData(typeof(CommandGetXdbcTypeInfo), "Command CommandGetXdbcTypeInfo not supported", true)]
public async void EnsureDoPutIsCorrectlyRoutedForTheCommand(Type commandType, string expectedResponse, bool isException = false)
public async Task EnsureDoPutIsCorrectlyRoutedForTheCommand(Type commandType, string expectedResponse, bool isException = false)
{
//Given
var command = (IMessage) Activator.CreateInstance(commandType)!;
var producer = new TestFlightSqlSever();
var descriptor = FlightDescriptor.CreateCommandDescriptor(command.PackAndSerialize().ToArray());
var recordBatch = new RecordBatch(new Schema(new List<Field>(), null), System.Array.Empty<Int8Array>(), 0);
var reader = new MockStreamReader<FlightData>(await recordBatch.ToFlightData(descriptor).ConfigureAwait(false));
var reader = new MockStreamReader<FlightData>(await recordBatch.ToFlightData(descriptor));
var batchReader = new FlightServerRecordBatchStreamReader(reader);
var mockStreamWriter = new MockServerStreamWriter<FlightPutResult>();

//When
async Task Act() => await producer.DoPut(batchReader, mockStreamWriter, new MockServerCallContext()).ConfigureAwait(false);
async Task Act() => await producer.DoPut(batchReader, mockStreamWriter, new MockServerCallContext());
var exception = await Record.ExceptionAsync(Act);
string? actualMessage = isException ? exception?.Message : mockStreamWriter.Messages[0].ApplicationMetadata.ToStringUtf8();

Expand All @@ -271,7 +272,7 @@ private class MockServerCallContext : ServerCallContext
protected override CancellationToken CancellationTokenCore => default;
protected override Metadata ResponseTrailersCore => new();
protected override Status StatusCore { get; set; }
protected override WriteOptions WriteOptionsCore { get; set; } = WriteOptions.Default;
protected override WriteOptions? WriteOptionsCore { get; set; } = WriteOptions.Default;
protected override AuthContext AuthContextCore => new("", new Dictionary<string, List<AuthProperty>>());
}
}
Expand Down Expand Up @@ -325,7 +326,7 @@ public static async Task<Schema> GetSchema(this IEnumerable<FlightData> flightDa
public static async Task<IEnumerable<FlightData>> ToFlightData(this RecordBatch recordBatch, FlightDescriptor? descriptor = null)
{
var responseStream = new MockFlightServerRecordBatchStreamWriter();
await responseStream.WriteRecordBatchAsync(recordBatch).ConfigureAwait(false);
await responseStream.WriteRecordBatchAsync(recordBatch);
if (descriptor == null)
{
return responseStream.FlightData;
Expand Down
30 changes: 15 additions & 15 deletions csharp/test/Apache.Arrow.Flight.Tests/FlightTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,9 @@ public async Task TestHandshake()
{
var duplexStreamingCall = _flightClient.Handshake();

await duplexStreamingCall.RequestStream.WriteAsync(new FlightHandshakeRequest(ByteString.Empty)).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.CompleteAsync().ConfigureAwait(false);
var results = await duplexStreamingCall.ResponseStream.ToListAsync().ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(new FlightHandshakeRequest(ByteString.Empty));
await duplexStreamingCall.RequestStream.CompleteAsync();
var results = await duplexStreamingCall.ResponseStream.ToListAsync();

Assert.Single(results);
Assert.Equal("Done", results.First().Payload.ToStringUtf8());
Expand All @@ -303,10 +303,10 @@ public async Task TestSingleExchange()
var duplexStreamingCall = _flightClient.DoExchange(flightDescriptor);
var expectedBatch = CreateTestBatch(0, 100);

await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.CompleteAsync().ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch);
await duplexStreamingCall.RequestStream.CompleteAsync();

var results = await duplexStreamingCall.ResponseStream.ToListAsync().ConfigureAwait(false);
var results = await duplexStreamingCall.ResponseStream.ToListAsync();

Assert.Single(results);
ArrowReaderVerifier.CompareBatches(expectedBatch, results.FirstOrDefault());
Expand All @@ -320,11 +320,11 @@ public async Task TestMultipleExchange()
var expectedBatch1 = CreateTestBatch(0, 100);
var expectedBatch2 = CreateTestBatch(100, 100);

await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch1).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch2).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.CompleteAsync().ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch1);
await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch2);
await duplexStreamingCall.RequestStream.CompleteAsync();

var results = await duplexStreamingCall.ResponseStream.ToListAsync().ConfigureAwait(false);
var results = await duplexStreamingCall.ResponseStream.ToListAsync();

ArrowReaderVerifier.CompareBatches(expectedBatch1, results[0]);
ArrowReaderVerifier.CompareBatches(expectedBatch2, results[1]);
Expand All @@ -338,8 +338,8 @@ public async Task TestExchangeWithMetadata()
var expectedBatch = CreateTestBatch(0, 100);
var expectedMetadata = ByteString.CopyFromUtf8("test metadata");

await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch, expectedMetadata).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.CompleteAsync().ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(expectedBatch, expectedMetadata);
await duplexStreamingCall.RequestStream.CompleteAsync();

List<ByteString> actualMetadata = new List<ByteString>();
List<RecordBatch> actualBatch = new List<RecordBatch>();
Expand All @@ -358,9 +358,9 @@ public async Task TestHandshakeWithSpecificMessage()
{
var duplexStreamingCall = _flightClient.Handshake();

await duplexStreamingCall.RequestStream.WriteAsync(new FlightHandshakeRequest(ByteString.CopyFromUtf8("Hello"))).ConfigureAwait(false);
await duplexStreamingCall.RequestStream.CompleteAsync().ConfigureAwait(false);
var results = await duplexStreamingCall.ResponseStream.ToListAsync().ConfigureAwait(false);
await duplexStreamingCall.RequestStream.WriteAsync(new FlightHandshakeRequest(ByteString.CopyFromUtf8("Hello")));
await duplexStreamingCall.RequestStream.CompleteAsync();
var results = await duplexStreamingCall.ResponseStream.ToListAsync();

Assert.Single(results);
Assert.Equal("Hello handshake", results.First().Payload.ToStringUtf8());
Expand Down
6 changes: 3 additions & 3 deletions csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,11 @@ public void ArrayAsCollection()
// Parameter 'values' must contain four values. The last value must be distinct from the rest.
private static void TestObjectArrayAsCollection<T, TArray>(TArray array, T nullValue, IReadOnlyList<T> values)
where T : class
where TArray : IArrowArray, ICollection<T?>
where TArray : IArrowArray, ICollection<T>
{
Assert.NotNull(array);
Assert.Equal(4, values.Count);
var collection = (ICollection<T?>)array;
var collection = (ICollection<T>)array;

Assert.Equal(array.Length, collection.Count);
Assert.Equal(4, collection.Count);
Expand All @@ -232,7 +232,7 @@ public void ArrayAsCollection()
Assert.False(collection.Contains(values[3]));

T sentinel = values[2];
T?[] destArr = { sentinel, sentinel, sentinel, sentinel, sentinel, sentinel };
T[] destArr = { sentinel, sentinel, sentinel, sentinel, sentinel, sentinel };
collection.CopyTo(destArr, 1);
Assert.Equal(sentinel, destArr[0]);
Assert.Equal(values[0], destArr[1]);
Expand Down
2 changes: 1 addition & 1 deletion csharp/test/Apache.Arrow.Tests/DurationArrayTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void AppendTimeSpanGivesSameTimeSpan(TimeSpan? timeSpan, DurationType typ
Assert.Equal(timeSpan, array.GetTimeSpan(0));

IReadOnlyList<TimeSpan?> asList = array;
Assert.Equal(1, asList.Count);
Assert.Single(asList);
Assert.Equal(timeSpan, asList[0]);
}
}
Expand Down

0 comments on commit e411e0e

Please sign in to comment.