Merge pull request #5117 from wolfsage/cassandane-tiny-strict-warnings

Cassandane::Tiny: Import strict/warnings to consumers
cyrusimap · Nov 15, 2024 · f15be89 · f15be89
2 parents 61cf3ce + b918c78
commit f15be89
Show file tree

Hide file tree

Showing 73 changed files with 190 additions and 162 deletions.
diff --git a/cassandane/Cassandane/BuildInfo.pm b/cassandane/Cassandane/BuildInfo.pm
@@ -38,6 +38,8 @@
 #
 
 package Cassandane::BuildInfo;
+use strict;
+use warnings;
 use JSON;
 
 use lib '.';

diff --git a/cassandane/Cassandane/Tiny.pm b/cassandane/Cassandane/Tiny.pm
@@ -5,6 +5,11 @@ use warnings;
 sub import {
   no warnings 'once';
   $Cassandane::Tiny::Loader::RELOADED = 1;
+
+  # Everyone gets strict and warnings
+  strict->import();
+  warnings->import();
+
   return;
 }
 

diff --git a/cassandane/Cassandane/Util/TestUrl.pm b/cassandane/Cassandane/Util/TestUrl.pm
@@ -1,5 +1,8 @@
 package Cassandane::Util::TestURL;
 
+use strict;
+use warnings;
+
 use Plack::Loader;
 use Plack::Request;
 use Plack::Response;

diff --git a/cassandane/tiny-tests/Caldav/invite_change_time b/cassandane/tiny-tests/Caldav/invite_change_time
@@ -53,31 +53,31 @@ EOF
     );
 
     xlog $self, "Change DURATION";
-    my ($href, $ical) = $create_event->();
+    ($href, $ical) = $create_event->();
     $ical =~ s/DURATION:PT1H/DURATION:PT2H/;
     $caldav->Request('PUT', $href, $ical, 'Content-Type' => 'text/calendar');
     $self->assert_caldav_notified(
         { recipient => '[email protected]', method => 'REQUEST' },
     );
 
     xlog $self, "Change DTSTART TZID to different UTC offset";
-    my ($href, $ical) = $create_event->();
+    ($href, $ical) = $create_event->();
     $ical =~ s/Perth/Melbourne/;
     $caldav->Request('PUT', $href, $ical, 'Content-Type' => 'text/calendar');
     $self->assert_caldav_notified(
         { recipient => '[email protected]', method => 'REQUEST' },
     );
 
     xlog $self, "Change TZID to same UTC offset using IANA alias";
-    my ($href, $ical) = $create_event->();
+    ($href, $ical) = $create_event->();
     $ical =~ s/Perth/West/;
     $caldav->Request('PUT', $href, $ical, 'Content-Type' => 'text/calendar');
     $self->assert_caldav_notified(
         { recipient => '[email protected]', method => 'REQUEST' },
     );
 
     xlog $self, "Change DURATION TO DTEND";
-    my ($href, $ical) = $create_event->();
+    ($href, $ical) = $create_event->();
     $ical =~ s/DURATION:PT1H/DTEND;TZID=Australia\/Perth:20240925T020000/;
     $caldav->Request('PUT', $href, $ical, 'Content-Type' => 'text/calendar');
     # Does not trigger an iTIP message, see Caldav.invite_switch_duration_to_dtend

diff --git a/cassandane/tiny-tests/Caldav/options b/cassandane/tiny-tests/Caldav/options
@@ -6,30 +6,30 @@ sub test_options {
 
     my $caldav = $self->{caldav};
 
-    sub get_options {
+    my $get_options = sub {
         my ($url) = shift;
         my $res = $caldav->ua()->request(
             'OPTIONS',
             $caldav->request_url($url),
             { headers => { Authorization => $caldav->auth_header() } }
         );
-        return $res->{headers}{dav};
-    }
+        return $res->{headers}{dav} || [];
+    };
 
     # Non-calendar collections - no scheduling
-    $self->assert_null(@{ get_options('/') });
+    $self->assert_null(@{ $get_options->('/') });
     $self->assert(not grep(/calendar-auto-schedule/,
-                           @{ get_options('/dav/') }));
+                           @{ $get_options->('/dav/') }));
 
     # Calendar collections - scheduling enabled by default
     $self->assert(grep(/calendar-auto-schedule/,
-            @{ get_options('/dav/calendars/') }));
+            @{ $get_options->('/dav/calendars/') }));
     $self->assert(grep(/calendar-auto-schedule/,
-            @{ get_options('/dav/calendars/user/') }));
+            @{ $get_options->('/dav/calendars/user/') }));
     $self->assert(grep(/calendar-auto-schedule/,
-            @{ get_options('/dav/calendars/user/cassandane/') }));
+            @{ $get_options->('/dav/calendars/user/cassandane/') }));
     $self->assert(grep(/calendar-auto-schedule/,
-            @{ get_options('/dav/calendars/user/cassandane/Default') }));
+            @{ $get_options->('/dav/calendars/user/cassandane/Default') }));
 
     # Disable scheduling on Default calendar
     my $xml = <<EOF;
@@ -49,5 +49,5 @@ EOF
 
     # Scheduling should NOT be advertised
     $self->assert(not grep(/calendar-auto-schedule/,
-            @{ get_options('/dav/calendars/user/cassandane/Default') }));
+            @{ $get_options->('/dav/calendars/user/cassandane/Default') }));
 }
diff --git a/cassandane/tiny-tests/Caldav/shared_multiget b/cassandane/tiny-tests/Caldav/shared_multiget
@@ -62,7 +62,7 @@ sub test_shared_multiget
 EOF
 
     xlog "Run calendar-multiget report";
-    $mgRes = $CalDAV->Request('REPORT', 'Default', $xmlMultiget,
+    my $mgRes = $CalDAV->Request('REPORT', 'Default', $xmlMultiget,
         'Content-Type' => 'application/xml',
     );
 

diff --git a/cassandane/tiny-tests/JMAPBackup/restore_contacts_all_dryrun b/cassandane/tiny-tests/JMAPBackup/restore_contacts_all_dryrun
@@ -63,8 +63,8 @@ sub test_restore_contacts_all_dryrun
     my $contactE = $res->[0][1]{created}{"e"}{id};
     my $state = $res->[0][1]{newState};
 
-    $diff = time() - $mark;
-    $period = "PT" . $diff . "S";
+    my $diff = time() - $mark;
+    my $period = "PT" . $diff . "S";
 
     xlog "restore contacts prior to most recent changes";
     $res = $jmap->CallMethods([['Backup/restoreContacts', {

diff --git a/cassandane/tiny-tests/JMAPBackup/restore_mail_twice b/cassandane/tiny-tests/JMAPBackup/restore_mail_twice
@@ -110,7 +110,7 @@ sub test_restore_mail_twice
     $self->assert_null($res->[1][1]{list}[0]{mailboxIds}->{$inbox->{id}});
 
     # need a gap between destroys otherwise we will restore both copies
-    my $mark = time();
+    $mark = time();
     sleep 2;
 
     xlog "destroy email1 again";

diff --git a/cassandane/tiny-tests/JMAPCalendars/admin_migrate39_defaultalerts b/cassandane/tiny-tests/JMAPCalendars/admin_migrate39_defaultalerts
@@ -92,7 +92,7 @@ EOF
 
     xlog $self, "Assert calendar default alerts";
 
-    $res = $jmap->CallMethods([
+    my $res = $jmap->CallMethods([
         ['Calendar/get', {
             properties => [
                 'defaultAlertsWithTime',
@@ -102,21 +102,22 @@ EOF
     ]);
     my %calendars = map { $_->{id} => $_ } @{$res->[0][1]{list}};
 
-    $self->assert_null($calendars{
-        Default}{defaultAlertsWithTime});
-    $self->assert_null($calendars{
-        Default}{defaultAlertsWithoutTime});
+    $self->assert_null($calendars{Default}{defaultAlertsWithTime});
+    $self->assert_null($calendars{Default}{defaultAlertsWithoutTime});
 
-    my $calendarADefaultAlerts = $calendars{$state->{
-        calendarA}{id}}{defaultAlertsWithTime};
+    my $calendarADefaultAlerts = $calendars{$state->{calendarA}{id}}
+                                           {defaultAlertsWithTime};
     $self->assert_num_equals(1, scalar keys %$calendarADefaultAlerts);
-    $self->assert_null($calendars{$state->{
-        calendarA}{id}}{defaultAlertsWithoutTime});
+    $self->assert_null(
+      $calendars{$state->{calendarA}{id}}{defaultAlertsWithoutTime}
+    );
 
-    $self->assert_null($calendars{$state->{
-        calendarB}{id}}{defaultAlertsWithTime});
-    $self->assert_null($calendars{$state->{
-        calendarB}{id}}{defaultAlertsWithoutTime});
+    $self->assert_null(
+      $calendars{$state->{calendarB}{id}}{defaultAlertsWithTime}
+    );
+    $self->assert_null(
+      $calendars{$state->{calendarB}{id}}{defaultAlertsWithoutTime}
+    );
 
     if (not $state->{did_migrate}) {
         $state->{calendarA}{defaultAlert} = (values %$calendarADefaultAlerts)[0];
@@ -287,7 +288,7 @@ EOF
         );
 
         xlog $self, "Create sharee per-user prop in calendar A";
-        my $ical = <<EOF;
+        $ical = <<EOF;
 BEGIN:VCALENDAR
 VERSION:2.0
 PRODID:-//Apple Inc.//Mac OS X 10.9.5//EN

diff --git a/cassandane/tiny-tests/JMAPCalendars/admin_rewrite_calendarevent_privacy b/cassandane/tiny-tests/JMAPCalendars/admin_rewrite_calendarevent_privacy
@@ -19,7 +19,7 @@ sub test_admin_rewrite_calendarevent_privacy
 );
 
     xlog $self, "Make sure regular user can't call Admin method";
-    $res = $jmap->CallMethods([
+    my $res = $jmap->CallMethods([
         ['Admin/rewriteCalendarEventPrivacy', {}, 'R1'],
     ], \@using);
     $self->assert_str_equals('accountNotSupportedByMethod',
@@ -28,7 +28,7 @@ sub test_admin_rewrite_calendarevent_privacy
     my $event1Uid = '40d11f36-245b-4a03-8034-df25f38f9f61';
 
     xlog $self, "create two calendar events";
-    my $res = $jmap->CallMethods([
+    $res = $jmap->CallMethods([
         ['CalendarEvent/set', {
             create => {
                 event1 => {

diff --git a/cassandane/tiny-tests/JMAPCalendars/calendar_set_defaultalerts_shared b/cassandane/tiny-tests/JMAPCalendars/calendar_set_defaultalerts_shared
@@ -12,11 +12,14 @@ sub test_calendar_set_defaultalerts_shared
     my $ownerJmap = $t->{owner}{jmap};
     my $shareeJmap = $t->{sharee}{jmap};
 
-    $self->assert_shared_defaultalerts($t);
+    # Using a string here will fail matching if we were to attempt to match,
+    # which we shouldn't, since there's no default alarms yet!
+    $self->assert_shared_defaultalerts($t, 'no alarm!', 'no alarm!');
 
     xlog $self, "Owner sets default alarms";
 
     my $alertWithTimeOwnerId = '4c08cb1d-60e0-46e0-9cc1-9622b7a820ed';
+
     $t->{owner}->{defaultAlertsWithTime} = {
         $alertWithTimeOwnerId => {
             '@type' => 'Alert',
@@ -55,7 +58,7 @@ sub test_calendar_set_defaultalerts_shared
         }, 'R1'],
     ]);
 
-    $self->assert_shared_defaultalerts($t);
+    $self->assert_shared_defaultalerts($t, $alertWithTimeOwnerId, 'no alarm!');
 
     xlog $self, 'Sharee sets default alarms';
     my $alertWithTimeShareeId = 'b61e5b53-8ea2-46f4-949d-7b49734ba4d3';
@@ -96,7 +99,7 @@ sub test_calendar_set_defaultalerts_shared
         }, 'R1'],
     ]);
 
-    $self->assert_shared_defaultalerts($t);
+    $self->assert_shared_defaultalerts($t, $alertWithTimeOwnerId, $alertWithTimeShareeId);
 
     xlog $self, 'Owner removes default alarms';
     $t->{owner}->{defaultAlertsWithTime} = undef;
@@ -115,7 +118,7 @@ sub test_calendar_set_defaultalerts_shared
         }, 'R1'],
     ]);
 
-    $self->assert_shared_defaultalerts($t);
+    $self->assert_shared_defaultalerts($t, 'no alarm!', $alertWithTimeShareeId);
 
     xlog $self, 'Sharee removes default alarms';
     $t->{sharee}->{defaultAlertsWithTime} = undef;
@@ -134,10 +137,10 @@ sub test_calendar_set_defaultalerts_shared
         }, 'R1'],
     ]);
 
-    $self->assert_shared_defaultalerts($t);
+    $self->assert_shared_defaultalerts($t, 'no alarm!', 'no alarm!');
 }
 
-sub _can_match {
+sub _can_match_b {
     my $event = shift;
     my $want = shift;
 
@@ -174,7 +177,7 @@ sub assert_alarm_notifs {
         my $found = 0;
         my @newwant;
         foreach my $data (@want) {
-            if (not $found and _can_match($event, $data)) {
+            if (not $found and _can_match_b($event, $data)) {
                 $found = 1;
             }
             else {
@@ -198,11 +201,13 @@ sub assert_alarm_notifs {
 
 sub assert_shared_defaultalerts
 {
-    my ($self, $t) = @_;
+    my ($self, $t, $owneralarmid, $shareealarmid) = @_;
 
     for my $who (qw/owner sharee/) {
         my $jmap = $t->{$who}->{jmap};
 
+        my $alarmid = $who eq 'owner' ? $owneralarmid : $shareealarmid;
+
         xlog $self, "Assert alarms for $who";
         my $res = $jmap->CallMethods([
             ['Calendar/get', {
@@ -242,8 +247,7 @@ sub assert_shared_defaultalerts
         $res = $caldav->Request('GET', $xhref);
         if ($defaultAlertsWithTime) {
             $self->assert_matches(qr/BEGIN:VALARM/, $res->{content});
-            my $uid = (values %{$defaultAlertsWithTime})->{uid};
-            $self->assert_matches(qr/UID:$uid/, $res->{content});
+            $self->assert_matches(qr/UID:$alarmid/, $res->{content});
         } else {
             $self->assert(not $res->{content} =~ qr/BEGIN:VALARM/);
         }
@@ -335,7 +339,7 @@ sub create_test
     $self->assert(exists $res->[1][1]{updated}{Default});
 
     xlog $self, 'Sharee sets useDefaultAlerts=true';
-    my $res = $shareeJmap->CallMethods([
+    $res = $shareeJmap->CallMethods([
         ['CalendarEvent/set', {
             accountId => 'owner',
             update => {

diff --git a/cassandane/tiny-tests/JMAPCalendars/calendar_set_shared_sort_order b/cassandane/tiny-tests/JMAPCalendars/calendar_set_shared_sort_order
@@ -50,7 +50,7 @@ sub test_calendar_set_shared_sort_order
     $self->assert_equals(JSON::false, $session->{accounts}{manifold}{isReadOnly});
 
     xlog $self, "Set sortOrder on a calendar";
-    $res = $jmap->CallMethods([
+    my $res = $jmap->CallMethods([
             ['Calendar/set', {
                     accountId => 'manifold',
                     update => {

diff --git a/cassandane/tiny-tests/JMAPCalendars/calendarevent_defaultalerts_calalarmd b/cassandane/tiny-tests/JMAPCalendars/calendarevent_defaultalerts_calalarmd
@@ -222,7 +222,7 @@ EOF
     $self->assert_alarms();
 }
 
-sub _can_match {
+sub _can_match_a {
     my $event = shift;
     my $want = shift;
 
@@ -260,7 +260,7 @@ sub assert_alarms {
         my $found = 0;
         my @newwant;
         foreach my $data (@want) {
-            if (not $found and _can_match($event, $data)) {
+            if (not $found and _can_match_a($event, $data)) {
                 $found = 1;
             }
             else {

diff --git a/cassandane/tiny-tests/JMAPCalendars/calendarevent_defaultalerts_etag_preserved_after_alarm b/cassandane/tiny-tests/JMAPCalendars/calendarevent_defaultalerts_etag_preserved_after_alarm
@@ -87,7 +87,7 @@ sub test_calendarevent_defaultalerts_etag_preserved_after_alarm
     $self->assert_num_equals(1, scalar @notifs);
 
     xlog $self, "Get event ETag after alarm fired";
-    my %Headers;
+    %Headers = ();
     if ($caldav->{user}) {
         $Headers{'Authorization'} = $caldav->auth_header();
     }