Changeset 207382 in webkit


Ignore:
Timestamp:
Oct 15, 2016 3:04:08 PM (7 years ago)
Author:
dbates@webkit.org
Message:

prepare-ChangeLog erroneously said that a python init method was deleted
https://bugs.webkit.org/show_bug.cgi?id=163456

Reviewed by Simon Fraser.

Fixes an issue where prepare-ChangeLog may list as deleted functions that are
immediately above added code.

Currently prepare-ChangeLog makes use of the same overlap detection algorithm
to compute the list of deleted functions as it does to compute added and modified
functions. We consider a function deleted if its entire function body and signature
are removed. It is sufficient to compare the list of functions before the patch
is applied and the list of functions are the patch is applied to identify
these functions.

  • Scripts/prepare-ChangeLog: Fix some style nits, including using Camel Case for

variable names.
(actuallyGenerateFunctionLists): Modified to call computeModifiedFunctions(). Always
compute the list of functions in the file after the patch regardless of whether the
patch only contains deletions. We will compare this list against the list of functions
before the patch was applied to determine the deleted functions.
(computeModifiedFunctions): Renamed; formerly named generateFunctionListsByRanges.
Removed out argument for the seen functions as we no longer make use of when computing
the list of deleted functions.
(diffCommand): Update comment.
(generateFunctionListsByRanges): Deleted.

  • Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl: Added more unit tests.
Location:
trunk/Tools
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Tools/ChangeLog

    r207370 r207382  
     12016-10-15  Daniel Bates  <dabates@apple.com>
     2
     3        prepare-ChangeLog erroneously said that a python __init__ method was deleted
     4        https://bugs.webkit.org/show_bug.cgi?id=163456
     5
     6        Reviewed by Simon Fraser.
     7
     8        Fixes an issue where prepare-ChangeLog may list as deleted functions that are
     9        immediately above added code.
     10
     11        Currently prepare-ChangeLog makes use of the same overlap detection algorithm
     12        to compute the list of deleted functions as it does to compute added and modified
     13        functions. We consider a function deleted if its entire function body and signature
     14        are removed. It is sufficient to compare the list of functions before the patch
     15        is applied and the list of functions are the patch is applied to identify
     16        these functions.
     17
     18        * Scripts/prepare-ChangeLog: Fix some style nits, including using Camel Case for
     19        variable names.
     20        (actuallyGenerateFunctionLists): Modified to call computeModifiedFunctions(). Always
     21        compute the list of functions in the file after the patch regardless of whether the
     22        patch only contains deletions. We will compare this list against the list of functions
     23        before the patch was applied to determine the deleted functions.
     24        (computeModifiedFunctions): Renamed; formerly named generateFunctionListsByRanges.
     25        Removed out argument for the seen functions as we no longer make use of when computing
     26        the list of deleted functions.
     27        (diffCommand): Update comment.
     28        (generateFunctionListsByRanges): Deleted.
     29        * Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl: Added more unit tests.
     30
    1312016-10-14  Simon Fraser  <simon.fraser@apple.com>
    232
  • trunk/Tools/Scripts/prepare-ChangeLog

    r205483 r207382  
    7070sub changeLogEmailAddressFromArgs($$);
    7171sub changeLogNameFromArgs($$);
     72sub computeModifiedFunctions($$$);
    7273sub createPatchCommand($$$$);
    7374sub decodeEntities($);
     
    8485sub generateFileList(\%$$$);
    8586sub generateFunctionLists($$$$$);
    86 sub generateFunctionListsByRanges($$$$);
    8787sub generateNewChangeLogs($$$$$$$$$$$$$$);
    8888sub getLatestChangeLogs($);
     
    350350        chomp $file;
    351351        $file =~ s/ /\\ /g;
     352
    352353        my %saw_function;
     354        my @afterChangeFunctionRanges;
     355
    353356        # Find all the functions in the file.
     357        my $sourceFileHandle = $delegateHashRef->{openFile}($file);
     358        next unless $sourceFileHandle;
     359        my @afterChangeFunctionRanges = get_function_line_ranges($sourceFileHandle, $file);
     360        close($sourceFileHandle);
     361
     362        # Find modified functions in the file.
    354363        if ($line_ranges_after_changed{$file}) {
    355             my $sourceFileHandle = $delegateHashRef->{openFile}($file);
    356             next unless $sourceFileHandle;
    357             my @function_ranges = get_function_line_ranges($sourceFileHandle, $file);
    358             close($sourceFileHandle);
    359 
    360364            my @change_ranges = (@{$line_ranges_after_changed{$file}}, []);
    361             my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@function_ranges, \%saw_function);
     365            my @functions = computeModifiedFunctions($file, \@change_ranges, \@afterChangeFunctionRanges);
    362366
    363367            # Format the list of functions.
     
    371375            my $originalFileHandle = $delegateHashRef->{openOriginalFile}($file, $gitCommit, $gitIndex, $mergeBase);
    372376            next unless $originalFileHandle;
    373             my @deleted_function_ranges = get_function_line_ranges($originalFileHandle, $file);
     377            my @beforeChangeFunctionRanges = get_function_line_ranges($originalFileHandle, $file);
    374378            close($originalFileHandle);
    375379
    376             my @change_ranges = (@{$line_ranges_before_changed{$file}}, []);
    377             my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@deleted_function_ranges, \%saw_function);
     380            my %existsAfterChange = map { $_->[2] => 1 } @afterChangeFunctionRanges;
     381
     382            my @functions;
     383            my %sawFunctions;
     384            for my $functionRange (@beforeChangeFunctionRanges) {
     385                my $functionName = $functionRange->[2];
     386                if (!$existsAfterChange{$functionName} && !$sawFunctions{$functionName}) {
     387                    push @functions, $functionName;
     388                    $sawFunctions{$functionName} = 1;
     389                }
     390            }
    378391
    379392            # Format the list of deleted functions.
     
    386399}
    387400
    388 sub generateFunctionListsByRanges($$$$)
    389 {
    390     my ($file, $changed_line_ranges, $function_ranges, $saw_function) = @_;
     401sub computeModifiedFunctions($$$)
     402{
     403    my ($file, $changedLineRanges, $functionRanges) = @_;
     404
     405    my %sawFunction;
    391406
    392407    # Find all the modified functions.
    393408    my @functions;
    394     my @change_ranges = @{$changed_line_ranges};
     409    my @change_ranges = @{$changedLineRanges};
    395410    my @change_range = (0, 0);
    396     FUNCTION: foreach my $function_range_ref (@{$function_ranges}) {
     411    FUNCTION: foreach my $function_range_ref (@{$functionRanges}) {
    397412        my @function_range = @{$function_range_ref};
    398413
     
    415430            if ($change_range[1] >= $function_range[0]
    416431                and $change_range[0] <= $function_range[1]) {
    417                 if (!$saw_function->{$function_range[2]}) {
    418                     $saw_function->{$function_range[2]} = 1;
     432                if (!$sawFunction{$function_range[2]}) {
     433                    $sawFunction{$function_range[2]} = 1;
    419434                    push @functions, $function_range[2];
    420435                }
     
    19972012    my ($paths, $gitCommit, $gitIndex, $mergeBase) = @_;
    19982013
    1999     # The function overlap detection logic in generateFunctionListsByRanges() assumes that its line
     2014    # The function overlap detection logic in computeModifiedFunctions() assumes that its line
    20002015    # ranges were from a unified diff without any context lines.
    20012016    my $command;
  • trunk/Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl

    r204969 r207382  
    7676EOF
    7777
     78my $EXAMPLE_JAVASCRIPT_WITH_SAME_FUNCTION_DEFINED_TWICE = <<EOF;
     79function f() {
     80    return 5;
     81}
     82
     83function f() {
     84    return 6;
     85}
     86
     87function g() {
     88    return 7;
     89}
     90EOF
     91
    7892my @testCaseHashRefs = (
    7993###
     
    146160
    147161        (fib): Deleted.
     162EOF
     163},
     164{
     165    testName => "Unified diff with 0 context; add function immediately after another function",
     166    inputText => $EXAMPLE_CPP,
     167    diffToApply => <<EOF,
     168diff --git a/fib.cpp b/fib.cpp
     169--- a/fib.cpp
     170+++ b/fib.cpp
     171@@ -11,0 +12,4 @@ unsigned fibPlusFive(unsigned n)
     172+unsigned fibPlusSeven(unsigned n)
     173+{
     174+    return fib(n) + 7;
     175+}
     176EOF
     177    expected => <<EOF
     178
     179        (fibPlusSeven):
     180EOF
     181},
     182{
     183    testName => "Unified diff with 0 context; add function at the end of the file",
     184    inputText => $EXAMPLE_CPP,
     185    diffToApply => <<EOF,
     186diff --git a/fib.cpp b/fib.cpp
     187--- a/fib.cpp
     188+++ b/fib.cpp
     189@@ -39,0 +40,5 @@ int main(int argc, const char* argv[])
     190+
     191+unsigned fibPlusSeven(unsigned n)
     192+{
     193+    return fib(n) + 7;
     194+}
     195EOF
     196    expected => <<EOF
     197
     198        (fibPlusSeven):
     199EOF
     200},
     201{
     202    testName => "Unified diff with 0 context; rename function",
     203    inputText => $EXAMPLE_CPP,
     204    diffToApply => <<EOF,
     205diff --git a/fib.cpp b/fib.cpp
     206--- a/fib.cpp
     207+++ b/fib.cpp
     208@@ -8 +8 @@ unsigned fib(unsigned);
     209-unsigned fibPlusFive(unsigned n)
     210+unsigned fibPlusFive2(unsigned n)
     211EOF
     212    expected => <<EOF
     213
     214        (fibPlusFive2):
     215        (fibPlusFive): Deleted.
     216EOF
     217},
     218{
     219    testName => "Unified diff with 0 context; replace function",
     220    inputText => $EXAMPLE_CPP,
     221    diffToApply => <<EOF,
     222diff --git a/fib.cpp b/fib.cpp
     223--- a/fib.cpp
     224+++ b/fib.cpp
     225@@ -8,4 +8,5 @@ unsigned fib(unsigned);
     226-unsigned fibPlusFive(unsigned n)
     227-{
     228-    return fib(n) + 5;
     229-}
     230+unsigned fibPlusSeven(unsigned n)
     231+{   // First comment
     232+    // Second comment
     233+    return fib(n) + 7;
     234+};
     235EOF
     236    expected => <<EOF
     237
     238        (fibPlusSeven):
     239        (fibPlusFive): Deleted.
     240EOF
     241},
     242{
     243    testName => "Unified diff with 0 context; move function from the top of the file to the end of the file",
     244    inputText => $EXAMPLE_CPP,
     245    diffToApply => <<EOF,
     246diff --git a/fib.cpp b/fib.cpp
     247--- a/fib.cpp
     248+++ b/fib.cpp
     249@@ -8,5 +7,0 @@ unsigned fib(unsigned);
     250-unsigned fibPlusFive(unsigned n)
     251-{
     252-    return fib(n) + 5;
     253-}
     254-
     255@@ -39,0 +35,5 @@ int main(int argc, const char* argv[])
     256+
     257+unsigned fibPlusFive(unsigned n)
     258+{
     259+    return fib(n) + 5;
     260+}
     261EOF
     262    expected => <<EOF
     263
     264        (fibPlusFive):
     265EOF
     266},
     267{
     268    testName => "Unified diff with 0 context; remove functions with the same name",
     269    inputText => $EXAMPLE_JAVASCRIPT_WITH_SAME_FUNCTION_DEFINED_TWICE,
     270    diffToApply => <<EOF,
     271diff --git a/fib.cpp b/fib.cpp
     272--- a/fib.cpp
     273+++ b/fib.cpp
     274@@ -1,8 +0,0 @@
     275-function f() {
     276-    return 5;
     277-}
     278-
     279-function f() {
     280-    return 6;
     281-}
     282-
     283EOF
     284    expected => <<EOF
     285
     286        (f): Deleted.
    148287EOF
    149288},
Note: See TracChangeset for help on using the changeset viewer.