Feature to select lines contained in [list of] SHAs/changelists.

henry2cox · henry2cox · commit b1cbb6d6193b · 2024-02-13T14:17:45.000-05:00
See 'genhtml --select-script ...'
Add file name and line number to 'select' callback.
Clarify '--select-script' documentation.

Signed-off-by: Henry Cox &lt;henry.cox@mediatek.com&gt;
diff --git a/bin/genhtml b/bin/genhtml
@@ -3586,35 +3586,99 @@ sub _read_udiff
 package InInterestingRegion;
 
 use constant {
-              NEXT  => 0,
-              PREV  => 1,
-              STACK => 2,
+              NEXT         => 0,
+              PREV         => 1,
+              STACK        => 2,
+              ANNOTATE_OBJ => 3,
+              LINECOV_HASH => 4,
+              # non-code line in a contiguous region before or after the
+              # 'interesting' code lie - should also be included -
+              # keep track of the length of that region, above and below
+              LINES_BEFORE => 5,
+              LINES_AFTER  => 6,
+
 };
 
 our $num_context_lines = 5;
 
 sub new
 {
-    my $class = shift;
-    my $self  = [undef, undef, Storable::dclone(\@_)];
+    my ($class, $srcFileStruct, $lineCovHash) = @_;
+
+    my $self = [undef, undef,
+                Storable::dclone($srcFileStruct->interesting_lines()),
+                $srcFileStruct, $lineCovHash
+    ];
+    # we will call the $annotateObj->line(int) to collect annotate data
+    #   for certain lines - to find out of a non-code line is in a
+    #   continguous region by the same author, in the same SHA or etc.
     $self->[NEXT] = shift(@{$self->[STACK]});
+    if (defined($self->[NEXT]) && $self->[NEXT] != 1) {
+        _computeContextBefore($self, 0);
+    }
     return bless $self, $class;
 }
 
+sub _computeContextBefore
+{
+    my ($self, $prev) = @_;
+
+    my $count    = 0;
+    my $data     = $self->[LINECOV_HASH];
+    my $annotate = $self->[ANNOTATE_OBJ];
+    # find contiguous source region which matches the select criteria.
+    #   That might include non-code lined (e.g., commments)
+    for (my $l = $self->[NEXT] - 1; $l > $prev; --$l) {
+        my $lineData     = $data->{$l} if exists($data->{$l});
+        my $annotateData = $annotate->line($l);
+        last
+            unless $selectCallback->select($lineData, $annotateData,
+                                           $self->[ANNOTATE_OBJ]->path(), $l);
+        ++$count;
+    }
+    # region of interest is the (possibly empty) contigous region we found
+    # plus the number of context lines
+    $self->[LINES_BEFORE] = $num_context_lines + $count;
+}
+
 sub interesting
 {
     my ($self, $lineNum) = @_;
+    die("unexpected lineNum '$lineNum'") if $lineNum < 1;
     if (defined($self->[NEXT])) {
         if ($lineNum == $self->[NEXT]) {
             $self->[PREV] = $self->[NEXT];
             $self->[NEXT] = shift(@{$self->[STACK]});
+            # compute following context lines - starting from here
+            #  and going to either end of file or next interesting line
+            my $max = defined($self->[NEXT]) ? $self->[NEXT] :
+                $self->[ANNOTATE_OBJ]->num_lines();
+            my $count    = 0;
+            my $data     = $self->[LINECOV_HASH];
+            my $annotate = $self->[ANNOTATE_OBJ];
+            for (my $l = $self->[PREV] + 1; $l < $max; ++$l) {
+                my $lineData     = $data->{$l} if exists($data->{$l});
+                my $annotateData = $annotate->line($l);
+                last
+                    if !$selectCallback->select($lineData, $annotateData,
+                                             $self->[ANNOTATE_OBJ]->path(), $l);
+                ++$count;
+            }
+            $self->[LINES_AFTER] = $num_context_lines + $count;
+
+            # and how many lines before the next interesting one?
+            if (defined($self->[NEXT])) {
+                $self->_computeContextBefore($self->[PREV]);
+            } else {
+                $self->[LINES_BEFORE] = 0;
+            }
             return 1;
-        } elsif ($lineNum >= $self->[NEXT] - $num_context_lines) {
+        } elsif ($lineNum >= $self->[NEXT] - $self->[LINES_BEFORE]) {
             return 1;
         }
     }
     if (defined($self->[PREV])) {
-        if ($lineNum <= $self->[PREV] + $num_context_lines) {
+        if ($lineNum <= $self->[PREV] + $self->[LINES_AFTER]) {
             return 1;
         }
         $self->[PREV] = undef;
@@ -3784,20 +3848,36 @@ sub new
     my @lineArray;    # interesting line numbers
     my $inRegion;
     if (defined($selectCallback)) {
+        # This implementation looks for all the 'line' coverpoints - then
+        #   expands the selected region around those points - e.g., by
+        #   adding noncode lines which are in the selected changelist and
+        #   surrounding the selected region with 'num_context_lines' of
+        #   context.
+        # A side effect of this criteria is that disjoint noncode regions -
+        #   e.g., comments or unused #ifdef code - will not be selected.
+        # A different implementation would go through the annotated source
+        #   and mark all selected lines, then go through again to add
+        #   context.
+        # It isn't clear which approach is best - but the current approach
+        #   seems to match user expectations - so I'm going with it, at
+        #   least for now.
+        #   Not too hard to do - but significant overkill - to make the
+        #   approach configurable.  Let's see if there is user demand.
         while (my ($line, $lne) = each(%$lineCovData)) {
             # ignore deleted lines as they don't appear in source listing
             my $tla = $lne->tla();
             # no annotations for deleted line
             my $annotateData = $self->line($line)
                 unless grep(/$tla/, ('DUB', 'DCB'));
             # callback arguments are (LineData, SourceLine)
-            if ($selectCallback->select($lne, $annotateData)) {
+            if ($selectCallback->select($lne, $annotateData,
+                                        $self->path(), $line)) {
                 push(@lineArray, $line);
             }
         }
         @lineArray                 = sort({ $a <=> $b } @lineArray);
         $self->[INTERESTING_LINES] = \@lineArray;
-        $inRegion                  = InInterestingRegion->new(@lineArray);
+        $inRegion = InInterestingRegion->new($self, $lineCovData);
     }
     foreach my $line (sort({
                                my $ka =
@@ -4176,6 +4256,12 @@ sub lines
     return $self->[LINES];
 }
 
+sub num_lines
+{
+    my $self = shift;
+    return scalar(@{$self->[LINES]});
+}
+
 sub is_empty
 {
     my $self = shift;
@@ -11141,7 +11227,7 @@ sub write_source($$$$$$$)
 
     my ($region, $empty);
     if ($selectCallback) {
-        $region = InInterestingRegion->new(@{$srcfile->interesting_lines()});
+        $region = InInterestingRegion->new($srcfile, $fileCovInfo->lineMap());
         $empty  = '';
         if ($srcfile->isProjectFile()) {
             $empty .= ' ' x $main::age_field_width;
diff --git a/lib/lcovutil.pm b/lib/lcovutil.pm
@@ -2633,11 +2633,13 @@ sub check_criteria
 
 sub select
 {
-    my ($self, $lineData, $annotateData) = @_;
+    my ($self, $lineData, $annotateData, $filename, $lineNo) = @_;
 
-    my @params = ('select', JsonSupport::encode($lineData));
-    push(@params, JsonSupport::encode($annotateData))
-        if defined($annotateData);
+    my @params = (
+               'select',
+               JsonSupport::encode($lineData),
+               defined($annotateData) ? JsonSupport::encode($annotateData) : '',
+               $filename, $lineNo);
     return $self->call(@params);
 }
 
diff --git a/man/lcovrc.5 b/man/lcovrc.5
@@ -2312,6 +2312,21 @@ The resolve script is called as:
 [callback_args]
 .I " file_name"
 .PP
+
+or
+.IP
+.I $resolve_callback =
+.B resolve_module
+.I ->new([callback_args])
+.PP
+to initialize the callback, then
+.IP
+.I $resolve_callback->
+.B resolve
+.I (file_name)
+.PP
+to find the actual file location.
+
 If necessary, the callback can check the suffix of the filename to determine
 whether it should look for either a source or data file.
 .PP
@@ -2337,29 +2352,56 @@ Lines which are not selected but fall within
 .I num_context_lines
 of a selected line are also included in the report.  See below.
 
+Note that selection is fundamentally intended to show regions of code with soem surrounding context.  It might not do what you expect if there is no code - e.g., if the region of interest has been compiled out via compiler or exclusion directives.
+For example: when selecting based on SHA or changelist ID, an inserted comment will not be selected unless it is
+.I num_context_lines
+of an inserted or changed line of code.
+
 The select script is called as:
 
 .B "   select_script"
 [callback_args]
-.I lineDataJson [annotateDataJson]
+.I lineDataJson annotateDataJson filenName lineNumber
 
 or as:
 
-.B "   select_module"
-[callback_args]
-.I lineDataRef [annotateDataRef]
+.I "   $selectCallback ="
+.B select_module
+.I ->new([callback_args])
+
+to initialize the callback object, then as
+
+.I "   " $selectCallback
+.B select
+.I  (lineDataRef annotateDataRef fileName lineNumber)
 
+.RS
+to determine selection,
 where
+.IP \- 3
+.I fileName
+is the name of the source file and
+.PP
+.IP \- 3
+.I lineNumber
+is the source file line number, indexed from zero,
+.PP
+.IP \- 3
 .I lineDataJson
-is a json-encoded LineData structure (see the lcovutil.pm source code) and
-.I annoateDataJson
+is a json-encoded LineData structure (see the lcovutil.pm source code), and
+.PP
+.IP \- 3
+.I annotateDataJson
 is the json-encoded data returned by your
 .I annotate\-script
 (see the
 .I \-\-annotate\-script
 parameter in man
 .B genhtml(1).
+), or the empty string if there are no annotations for this file.
+.PP
 The module callback is similar except that is is passed objects rather than JSON encodings of the objects.
+.RE
 
 The script should return "1" or "0".
 
diff --git a/scripts/select.pm b/scripts/select.pm
