Skip to content

Commit

Permalink
Try different error for missing yard docs (#570)
Browse files Browse the repository at this point in the history
Co-authored-by: David Mora <[email protected]>
  • Loading branch information
dgmora and davidgm0 authored Jul 11, 2022
1 parent ef40714 commit 87f6275
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 14 deletions.
4 changes: 4 additions & 0 deletions lib/solargraph/api_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class ApiMap
# @return [Array<String>]
attr_reader :unresolved_requires

# @return [Array<String>]
attr_reader :missing_docs

# @param pins [Array<Solargraph::Pin::Base>]
def initialize pins: []
@source_map_hash = {}
Expand Down Expand Up @@ -68,6 +71,7 @@ def catalog bench
yard_map.change(external_requires, bench.workspace.directory, bench.workspace.source_gems)
@store = Store.new(yard_map.pins + implicit.pins + pins)
@unresolved_requires = yard_map.unresolved_requires
@missing_docs = yard_map.missing_docs
@rebindable_method_names = nil
store.block_pins.each { |blk| blk.rebind(self) }
self
Expand Down
16 changes: 16 additions & 0 deletions lib/solargraph/diagnostics/require_not_found.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def diagnose source, api_map
refs = {}
map = api_map.source_map(source.filename)
map.requires.each { |ref| refs[ref.name] = ref }
api_map.missing_docs.each do |r|
next unless refs.key?(r)
result.push docs_error(r, refs[r].location)
end
api_map.unresolved_requires.each do |r|
next unless refs.key?(r)
result.push require_error(r, refs[r].location)
Expand All @@ -21,6 +25,18 @@ def diagnose source, api_map

private

# @param path [String]
# @param location [Location]
# @return [Hash]
def docs_error path, location
{
range: location.range.to_hash,
severity: Diagnostics::Severities::WARNING,
source: 'RequireNotFound',
message: "Yard docs missing for #{path}"
}
end

# @param path [String]
# @param location [Location]
# @return [Hash]
Expand Down
43 changes: 29 additions & 14 deletions lib/solargraph/yard_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ def unresolved_requires
@unresolved_requires ||= []
end

# @return [Array<String>]
def missing_docs
@missing_docs ||= []
end

# @param y [String]
# @return [YARD::Registry]
def load_yardoc y
Expand Down Expand Up @@ -196,10 +201,11 @@ def process_requires
required.merge @gemset.keys if required.include?('bundler/require')
pins.replace core_pins
unresolved_requires.clear
missing_docs.clear
stdlib_pins.clear
environ = Convention.for_global(self)
done = []
from_std = []
already_errored = []
(required + environ.requires).each do |r|
next if r.nil? || r.empty? || done.include?(r)
done.push r
Expand All @@ -219,24 +225,18 @@ def process_requires
# YARD detects gems for certain libraries that do not have a yardoc
# but exist in the stdlib. `fileutils` is an example. Treat those
# cases as errors and check the stdlib yardoc.
raise Gem::LoadError if yd.nil?
if yd.nil?
process_error(r, result, already_errored, nil)
next
end
@gem_paths[spec.name] = spec.full_gem_path
unless yardocs.include?(yd)
yardocs.unshift yd
result.concat process_yardoc yd, spec
result.concat add_gem_dependencies(spec) if with_dependencies?
end
rescue Gem::LoadError, NoYardocError => e
base = r.split('/').first
next if from_std.include?(base)
from_std.push base
stdtmp = load_stdlib_pins(base)
if stdtmp.empty?
unresolved_requires.push r
else
stdlib_pins.concat stdtmp
result.concat stdtmp
end
rescue Gem::LoadError, NoYardocError
process_error(r, result, already_errored)
end
result.delete_if(&:nil?)
unless result.empty?
Expand All @@ -253,6 +253,21 @@ def process_requires
pins.concat environ.pins
end

def process_error(req, result, already_errored, yd = 1)
base = req.split('/').first
return if already_errored.include?(base)
already_errored.push base
stdtmp = load_stdlib_pins(base)
if yd.nil?
missing_docs.push req
elsif stdtmp.empty?
unresolved_requires.push req
else
stdlib_pins.concat stdtmp
result.concat stdtmp
end
end

def process_gemsets
return {} if directory.empty? || !File.file?(File.join(directory, 'Gemfile'))
require_from_bundle(directory)
Expand All @@ -270,7 +285,7 @@ def add_gem_dependencies spec
@gem_paths[depspec.name] = depspec.full_gem_path
gy = yardoc_file_for_spec(depspec)
if gy.nil?
unresolved_requires.push dep.name
missing_docs.push dep.name
else
next if yardocs.include?(gy)
yardocs.unshift gy
Expand Down
6 changes: 6 additions & 0 deletions spec/yard_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@
expect(yard_map.unresolved_requires).not_to include('set')
end

it "tracks missing documentation" do
yard_map = Solargraph::YardMap.new(required: ['set', 'not_valid'])
expect(yard_map.requires_missing_documentation).to include('not_valid')
expect(yard_map.requires_missing_documentation).not_to include('set')
end

it "ignores duplicate requires" do
# Assuming the parser gem exists because it's a Solargraph dependency
yard_map = Solargraph::YardMap.new(required: ['parser', 'parser'])
Expand Down

0 comments on commit 87f6275

Please sign in to comment.