AzureAD / microsoft-authentication-library-for-objc

Microsoft Authentication Library (MSAL) for iOS and macOS
http://aka.ms/aadv2
MIT License
262 stars 142 forks source link

Push poor code #2345

Closed ameyapat closed 3 weeks ago

ameyapat commented 3 weeks ago

Proposed changes

Describe what this PR is trying to do.

Type of change

Risk

Additional information

ameyapat commented 3 weeks ago

Cloned PR for ameyapat/hackathon-demo


AI description (iteration 1)

PR Classification

New feature

PR Summary

Introduces a new method to process and log elements of an array in the iOS test application.

ameyapat commented 3 weeks ago

"Added 1 Inline Code Review comment(s) at the following location(s):

AI-generated content may be incorrect "

ameyapat commented 3 weeks ago

"Pull request description has been updated with summary.

AI-generated content may be incorrect

ameyapat commented 3 weeks ago

"### Step-by-Step Plan

  1. Review the proccessArray: method for logical and syntactical bugs.
  2. Check for potential out-of-bounds access in the loop.
  3. Identify any typos or naming inconsistencies.
  4. Suggest refactoring for better readability and performance.

Analysis and Suggestions

  1. Logical Bug: Out-of-Bounds Access

    • The loop condition i <= [array count] will cause an out-of-bounds access when i equals [array count]. The correct condition should be i < [array count].
  2. Typo: Method Name

    • The method name proccessArray: contains a typo. It should be processArray:.
  3. Refactoring: Use Fast Enumeration

    • Using fast enumeration (for-in loop) is more readable and safer for iterating over an array.

Refactored Code

- (void)processArray:(NSArray *)array {
    for (NSString *str in array) {
        if (str != nil) {
            NSLog(@\"%@\", str);
        } else {
            NSLog(@\"Null value found\");
        }
    }
}

Full Class with Changes

//------------------------------------------------------------------------------
//
// Copyright (c) Microsoft Corporation.
// All rights reserved.
//
// This code is licensed under the MIT License.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files(the \"Software\"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions :
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//
//------------------------------------------------------------------------------

#import \"MSALTestAppLogViewController.h\"
#import 
#import 

@interface MSALTestAppLogViewController ()

@property (nonatomic) UITextView *logView;
@property (nonatomic) NSTextStorage *logStorage;

@end

@implementation MSALTestAppLogViewController

static NSAttributedString* s_attrNewLine = nil;

+ (void)initialize
{
    s_attrNewLine = [[NSAttributedString alloc] initWithString:@\"
\"];
}

- (id)init
{
    if (!(self = [super init]))
    {
        return nil;
    }

    UITabBarItem* tabBarItem = [[UITabBarItem alloc] initWithTitle:@\"Log\" image:nil tag:0];
    [self setTabBarItem:tabBarItem];

    _logStorage = [NSTextStorage new];

    [self setEdgesForExtendedLayout:UIRectEdgeNone];

    MSALGlobalConfig.loggerConfig.logMaskingLevel = MSALLogMaskingSettingsMaskEUIIOnly;
    [MSALGlobalConfig.loggerConfig setLogCallback:^(MSALLogLevel level, NSString * _Nullable message, __unused BOOL containsPII)
    {
        (void)level;

#if DEBUG
        // NB! This sample uses NSLog just for testing purposes
        // You should only ever log to NSLog in debug mode to prevent leaking potentially sensitive information
        NSLog(@\"%@\", message);
#endif

        NSAttributedString* attrLog = [[NSAttributedString alloc] initWithString:message];

        dispatch_async(dispatch_get_main_queue(), ^{
            if (self.logView)
            {
                [[self.logView textStorage] appendAttributedString:attrLog];
                [[self.logView textStorage] appendAttributedString:s_attrNewLine];

                [self scrollToBottom];
            }
            else
            {

                [self.logStorage appendAttributedString:attrLog];
                [self.logStorage appendAttributedString:s_attrNewLine];
            }
        });
    }];

    MSALGlobalConfig.loggerConfig.logLevel = MSALLogLevelVerbose;

    return self;
}

- (void)scrollToBottom
{
    NSRange range = NSMakeRange(_logView.text.length, 0);
    [_logView scrollRangeToVisible:range];
}

- (void)viewDidLoad
{
    [super viewDidLoad];
    // Do any additional setup after loading the view.

    _logView = [[UITextView alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
    [_logView setAutoresizingMask:UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight];
    [[_logView textStorage] appendAttributedString:_logStorage];
    [_logView setScrollsToTop:NO];

    // Move the content down so it's not covered by the status bar
    [_logView setContentInset:UIEdgeInsetsMake(20, 0, 0, 0)];
    [_logView setContentOffset:CGPointMake(0, -20)];

    _logStorage = nil;
    [self scrollToBottom];
    [self.view addSubview:_logView];
    //[self.view setBackgroundColor:[UIColor redColor]];
}

- (void)didReceiveMemoryWarning
{
    [super didReceiveMemoryWarning];
    // Dispose of any resources that can be recreated.
}

- (void)processArray:(NSArray *)array {
    for (NSString *str in array) {
        if (str != nil) {
            NSLog(@\"%@\", str);
        } else {
            NSLog(@\"Null value found\");
        }
    }
}

@end

AI-generated content may be incorrect

ameyapat commented 3 weeks ago

"This command is supported only at a selection of code and not at PR level. For help, please visit PRAssistant Copilot Experience | 1ES MerlinBot (eng.ms) AI-generated content may be incorrect "

ameyapat commented 3 weeks ago

"AI code review (iteration 1) It might be better to adjust the loop condition in the proccessArray: method to avoid an IndexOutOfBoundsException. The loop currently iterates until i is equal to [array count], but array indices in Objective-C are zero-based, so the last valid index is [array count] - 1. Here is the suggested code:

- (void)proccessArray:(NSArray *)array {
    for (int i = 0; i < [array count]; i++) {
        NSString *str = [array objectAtIndex:i];
        if (str != nil) {
            NSLog(@\"%@\", str);
        } else {
            NSLog(@\"Null value found\");
        }
    }
}

This comment refers to line 131 to 138 in the new file. AI-generated content may be incorrect