Skip to content

Commit

Permalink
[tests] Add a test to verify we don't add constructors with 'out NSEr…
Browse files Browse the repository at this point in the history 
…ror' parameters. (#21740)

A constructor can't fail in managed code (without throwing an exception), so
having an 'out NSError' parameter is useless.
  • Loading branch information
@rolfbjarne
rolfbjarne authored Dec 3, 2024
1 parent 6ef1531 commit b53a5c0
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 0 deletions.
84 changes: 84 additions & 0 deletions tests/cecil-tests/ConstructorTest.KnownFailures.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,89 @@ public partial class ConstructorTest {
"UIKit.UIImageStatusDispatcher::.ctor(UIKit.UIImage/SaveStatus)",
"UIKit.UIVideoStatusDispatcher::.ctor(UIKit.UIVideo/SaveStatus)",
};

static HashSet<string> knownFailuresCtorsWithOutNSErrorParameter = new HashSet<string> {
"AppKit.NSDocument::.ctor(Foundation.NSUrl,Foundation.NSUrl,System.String,Foundation.NSError&)",
"AppKit.NSDocument::.ctor(Foundation.NSUrl,System.String,Foundation.NSError&)",
"AppKit.NSDocument::.ctor(System.String,Foundation.NSError&)",
"ARKit.ARReferenceObject::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"AudioUnit.AUAudioUnit::.ctor(AudioUnit.AudioComponentDescription,AudioUnit.AudioComponentInstantiationOptions,Foundation.NSError&)",
"AudioUnit.AUAudioUnit::.ctor(AudioUnit.AudioComponentDescription,Foundation.NSError&)",
"AudioUnit.AUAudioUnitBus::.ctor(AVFoundation.AVAudioFormat,Foundation.NSError&)",
"AVFoundation.AVAssetReader::.ctor(AVFoundation.AVAsset,Foundation.NSError&)",
"AVFoundation.AVAssetWriter::.ctor(Foundation.NSUrl,System.String,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,AVFoundation.AudioSettings,AVFoundation.AVAudioCommonFormat,System.Boolean,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,AVFoundation.AudioSettings,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,AVFoundation.AVAudioCommonFormat,System.Boolean,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,Foundation.NSDictionary,AVFoundation.AVAudioCommonFormat,System.Boolean,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,Foundation.NSDictionary,Foundation.NSError&)",
"AVFoundation.AVAudioFile::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"AVFoundation.AVAudioPlayer::.ctor(Foundation.NSData,System.String,Foundation.NSError&)",
"AVFoundation.AVAudioPlayer::.ctor(Foundation.NSUrl,System.String,Foundation.NSError&)",
"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AudioSettings,Foundation.NSError&)",
"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AVAudioFormat,Foundation.NSError&)",
"AVFoundation.AVCaptureDeviceInput::.ctor(AVFoundation.AVCaptureDevice,Foundation.NSError&)",
"AVFoundation.AVMidiPlayer::.ctor(Foundation.NSData,Foundation.NSUrl,Foundation.NSError&)",
"AVFoundation.AVMidiPlayer::.ctor(Foundation.NSUrl,Foundation.NSUrl,Foundation.NSError&)",
"AVFoundation.AVMutableMovie::.ctor(AVFoundation.AVMovie,Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSObject>,Foundation.NSError&)",
"AVFoundation.AVMutableMovie::.ctor(Foundation.NSData,Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSObject>,Foundation.NSError&)",
"AVFoundation.AVMutableMovie::.ctor(Foundation.NSUrl,Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSObject>,Foundation.NSError&)",
"BrowserEngineKit.BEMediaEnvironment::.ctor(Foundation.NSObject,Foundation.NSError&)",
"CoreHaptics.CHHapticEngine::.ctor(AVFoundation.AVAudioSession,Foundation.NSError&)",
"CoreHaptics.CHHapticEngine::.ctor(Foundation.NSError&)",
"CoreHaptics.CHHapticPattern::.ctor(CoreHaptics.CHHapticEvent[],CoreHaptics.CHHapticDynamicParameter[],Foundation.NSError&)",
"CoreHaptics.CHHapticPattern::.ctor(CoreHaptics.CHHapticEvent[],CoreHaptics.CHHapticParameterCurve[],Foundation.NSError&)",
"CoreHaptics.CHHapticPattern::.ctor(CoreHaptics.CHHapticPatternDefinition,Foundation.NSError&)",
"CoreHaptics.CHHapticPattern::.ctor(Foundation.NSDictionary,Foundation.NSError&)",
"CoreHaptics.CHHapticPattern::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"CoreML.MLArrayBatchProvider::.ctor(Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSArray>,Foundation.NSError&)",
"CoreML.MLCustomModel::.ctor(CoreML.MLModelDescription,Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSObject>,Foundation.NSError&)",
"CoreML.MLDictionaryFeatureProvider::.ctor(Foundation.NSDictionary`2<Foundation.NSString,Foundation.NSObject>,Foundation.NSError&)",
"CoreML.MLMultiArray::.ctor(Foundation.NSNumber[],CoreML.MLMultiArrayDataType,Foundation.NSError&)",
"CoreML.MLMultiArray::.ctor(System.IntPtr,Foundation.NSNumber[],CoreML.MLMultiArrayDataType,Foundation.NSNumber[],System.Action`1<System.IntPtr>,Foundation.NSError&)",
"CoreML.MLMultiArray::.ctor(System.IntPtr,System.IntPtr[],CoreML.MLMultiArrayDataType,System.IntPtr[],System.Action`1<System.IntPtr>,Foundation.NSError&)",
"CoreML.MLMultiArray::.ctor(System.IntPtr[],CoreML.MLMultiArrayDataType,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSData,Foundation.NSAttributedStringDocumentAttributes,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSData,Foundation.NSAttributedStringMarkdownParsingOptions,Foundation.NSUrl,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSData,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSUrl,Foundation.NSAttributedStringDocumentAttributes,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSUrl,Foundation.NSAttributedStringMarkdownParsingOptions,Foundation.NSUrl,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"Foundation.NSAttributedString::.ctor(System.String,Foundation.NSAttributedStringMarkdownParsingOptions,Foundation.NSUrl,Foundation.NSError&)",
"Foundation.NSDataDetector::.ctor(Foundation.NSTextCheckingType,Foundation.NSError&)",
"Foundation.NSDataDetector::.ctor(Foundation.NSTextCheckingTypes,Foundation.NSError&)",
"Foundation.NSDictionary::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"Foundation.NSFileWrapper::.ctor(Foundation.NSUrl,Foundation.NSFileWrapperReadingOptions,Foundation.NSError&)",
"Foundation.NSKeyedUnarchiver::.ctor(Foundation.NSData,Foundation.NSError&)",
"Foundation.NSRegularExpression::.ctor(Foundation.NSString,Foundation.NSRegularExpressionOptions,Foundation.NSError&)",
"Foundation.NSUrl::.ctor(Foundation.NSData,Foundation.NSUrlBookmarkResolutionOptions,Foundation.NSUrl,System.Boolean&,Foundation.NSError&)",
"GLKit.GLKMesh::.ctor(ModelIO.MDLMesh,Foundation.NSError&)",
"HealthKit.HKAudiogramSensitivityTest::.ctor(HealthKit.HKQuantity,HealthKit.HKAudiogramConductionType,System.Boolean,HealthKit.HKAudiogramSensitivityTestSide,HealthKit.HKAudiogramSensitivityPointClampingRange,Foundation.NSError&)",
"HealthKit.HKWorkoutSession::.ctor(HealthKit.HKHealthStore,HealthKit.HKWorkoutConfiguration,Foundation.NSError&)",
"HealthKit.HKWorkoutSession::.ctor(HealthKit.HKWorkoutConfiguration,Foundation.NSError&)",
"iTunesLibrary.ITLibrary::.ctor(System.String,Foundation.NSError&)",
"iTunesLibrary.ITLibrary::.ctor(System.String,iTunesLibrary.ITLibInitOptions,Foundation.NSError&)",
"Messages.MSSticker::.ctor(Foundation.NSUrl,System.String,Foundation.NSError&)",
"MetalKit.MTKMesh::.ctor(ModelIO.MDLMesh,Metal.IMTLDevice,Foundation.NSError&)",
"MetalPerformanceShaders.MPSKeyedUnarchiver::.ctor(Foundation.NSData,Metal.IMTLDevice,Foundation.NSError&)",
"ModelIO.MDLAsset::.ctor(Foundation.NSUrl,ModelIO.MDLVertexDescriptor,ModelIO.IMDLMeshBufferAllocator,System.Boolean,Foundation.NSError&)",
"NaturalLanguage.NLGazetteer::.ctor(Foundation.NSData,Foundation.NSError&)",
"NaturalLanguage.NLGazetteer::.ctor(Foundation.NSDictionary,Foundation.NSString,Foundation.NSError&)",
"NaturalLanguage.NLGazetteer::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"NaturalLanguage.NLGazetteer::.ctor(NaturalLanguage.NLStrongDictionary,System.Nullable`1<NaturalLanguage.NLLanguage>,Foundation.NSError&)",
"NearbyInteraction.NINearbyAccessoryConfiguration::.ctor(Foundation.NSData,Foundation.NSError&)",
"NearbyInteraction.NINearbyAccessoryConfiguration::.ctor(Foundation.NSData,Foundation.NSUuid,Foundation.NSError&)",
"PassKit.PKAddPassesViewController::.ctor(Foundation.NSData,Foundation.NSData,Foundation.NSError&)",
"PassKit.PKPass::.ctor(Foundation.NSData,Foundation.NSError&)",
"PencilKit.PKDrawing::.ctor(Foundation.NSData,Foundation.NSError&)",
"Phase.PhaseSoundEvent::.ctor(Phase.PhaseEngine,System.String,Foundation.NSError&)",
"Phase.PhaseSoundEvent::.ctor(Phase.PhaseEngine,System.String,Phase.PhaseMixerParameters,Foundation.NSError&)",
"ScreenTime.STWebHistory::.ctor(System.String,Foundation.NSError&)",
"ShazamKit.SHCustomCatalog::.ctor(Foundation.NSData,Foundation.NSError&)",
"ShazamKit.SHSignature::.ctor(Foundation.NSData,Foundation.NSError&)",
"SoundAnalysis.SNAudioFileAnalyzer::.ctor(Foundation.NSUrl,Foundation.NSError&)",
"SoundAnalysis.SNClassifySoundRequest::.ctor(CoreML.MLModel,Foundation.NSError&)",
"SoundAnalysis.SNClassifySoundRequest::.ctor(System.String,Foundation.NSError&)",
};
}
}
37 changes: 37 additions & 0 deletions tests/cecil-tests/ConstructorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,5 +518,42 @@ public void NonDefaultCtorDoesNotCallBaseDefaultCtor ()
nameof (knownFailuresNonDefaultCtorDoesNotCallBaseDefaultCtor),
"Non-default ctors that call the base class' default ctor.", (v) => $"{v.Location}: {v.Message}");
}

[Test]
public void NoConstructorsWithOutErrorArguments ()
{
Configuration.IgnoreIfAnyIgnoredPlatforms ();

var failures = new Dictionary<string, FailureWithMessageAndLocation> ();
foreach (var info in Helper.NetPlatformImplementationAssemblyDefinitions) {
foreach (var type in info.Assembly.MainModule.Types) {
if (!type.IsClass || !type.HasMethods)
continue;

if (!SubclassesNSObject (type))
continue;

foreach (var ctor in type.Methods.Where (v => !v.IsStatic && v.IsConstructor && v.HasParameters && v.Parameters.Count > 0)) {
// Presumably obsolete members have been obsoleted for a reason, so assume there's a correctly bound version.
if (ctor.IsObsolete ())
continue;

foreach (var param in ctor.Parameters) {
if (param.ParameterType is not ByReferenceType)
continue;
if (!param.ParameterType.GetElementType ().Is ("Foundation", "NSError"))
continue;
var msg = $"{ctor.RenderMethod ()}: This constructor has an 'out NSError' parameter. Such constructors should be bound as factory methods instead.";
failures [ctor.RenderMethod ()] = new FailureWithMessageAndLocation (msg, GetLocation (ctor));
continue;
}
}
}
}
Helper.AssertFailures (failures,
knownFailuresCtorsWithOutNSErrorParameter,
nameof (knownFailuresCtorsWithOutNSErrorParameter),
"Constructors with 'out NSError' parameters", (v) => $"{v.Location}: {v.Message}");
}
}
}

8 comments on commit b53a5c0

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

Please sign in to comment.