Fix critical exception handling and Ruby idiom violations #5531

Copilot · 2025-11-19T04:56:41Z

Pull request overview

Addresses critical security and code quality issues flagged by Ultimate Bug Scanner: improper exception handling, resource leaks, and non-idiomatic nil checks.

Changes

Exception Handling (13 instances)

rescue Exception → rescue StandardError to prevent catching system signals
Bare rescue → rescue StandardError for explicit error handling
Files: measure_manager_server.rb, embedded_help.rb, measure_manager_test.rb, openstudio_cli.rb, measure_manager.rb

Resource Management (1 instance)

BlameFiles.rb: File handle leak fixed using block form of File.open

# Before
infile = File.open(file, "r")
infile.each_line do |line|
  # ...
end
# file never closed

# After
File.open(file, "r") do |infile|
  infile.each_line do |line|
    # ...
  end
end

Nil Checks (30+ instances)

== nil / != nil → .nil? / !.nil?
Files: SwigWrapToRDoc.rb, openstudio_cli.rb, Polygon3d_Join_Test.rb, others

Not Changed (intentional)

Marshal.load in tests (testing Marshal functionality)
eval usage (required for CLI dynamic loading)
Global variables (existing architecture)

Pull Request Author

All new and existing tests passes

Labels:

If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
CI status: all green or justified

Original prompt

This section details on the original issue you should resolve

<issue_title>Bugs from Ultimate Bug Scanner</issue_title>
<issue_description>Issue overview

Project: /Users/achapin/OpenStudio/openstudio-full/OpenStudio
Started: 2025-11-19T04:48:14Z
Files: 0 source files (rb,rake,ru,gemspec,erb,haml,slim,rbi,rbs,jbuilder)

✓ ast-grep available (ast-grep) - full AST analysis enabled
⚠ Bundler or Gemfile not detected - will run tools if globally installed

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

NIL / DEFENSIVE PROGRAMMING
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

▓▓▓ Detects: nil equality, deep method chains without guards, dig? usage
Prefer x.nil?, safe navigation (&.), and Hash#dig to avoid NoMethodError.

• == nil or != nil (prefer .nil?)
⚠ Warning (30 found)
Equality to nil
Use x.nil? / !x.nil?
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/SwigWrapToRDoc.rb:61 (

OpenStudio/developer/ruby/SwigWrapToRDoc.rb

Line 61 in c7f13ad

if strArray[i].index(/SWIGEXPORT void Init_/) != nil then

)
if strArray[i].index(/SWIGEXPORT void Init_/) != nil then
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/SwigWrapToRDoc.rb:72 (

OpenStudio/developer/ruby/SwigWrapToRDoc.rb

Line 72 in c7f13ad

if strArray[i].index(/Document-[\w]*: /) == nil then

)
if strArray[i].index(/Document-[\w]*: /) == nil then
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/SwigWrapToRDoc.rb:79 (

OpenStudio/developer/ruby/SwigWrapToRDoc.rb

Line 79 in c7f13ad

if ans != nil then

)
if ans != nil then
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/SwigWrapToRDoc.rb:105 (

OpenStudio/developer/ruby/SwigWrapToRDoc.rb

Line 105 in c7f13ad

if (ln.match(/static VALUE/) != nil) ||

)
if (ln.match(/static VALUE/) != nil) ||
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/SwigWrapToRDoc.rb:106 (

OpenStudio/developer/ruby/SwigWrapToRDoc.rb

Line 106 in c7f13ad

(ln.match(/_wrap_/) != nil) ||

)
(ln.match(/wrap/) != nil) ||

• Deep method chains (use &. / guards)
ℹ Info (307 found)
Fragile deep chaining
Consider &. or guard clauses
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/ruby/engine/measure_manager_server.rb:163 (

OpenStudio/ruby/engine/measure_manager_server.rb

Line 163 in c7f13ad

OpenStudio::LocalBCL.instance.measures.each do |local_measure|

)
OpenStudio::LocalBCL.instance.measures.each do |local_measure|
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/ruby/engine/embedded_help.rb:28 (

OpenStudio/ruby/engine/embedded_help.rb

Line 28 in c7f13ad

if p.to_s.chars.first == ':' then

)
if p.to_s.chars.first == ':' then
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/ruby/engine/embedded_help.rb:183 (

OpenStudio/ruby/engine/embedded_help.rb

Line 183 in c7f13ad

elsif path_with_extension.to_s.chars.first == ':'

)
elsif path_with_extension.to_s.chars.first == ':'

• Hash#[] chained without dig
ℹ Info (96 found)
Nested [] access
Consider Hash#dig(:a,:b)

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
2. NUMERIC / ARITHMETIC PITFALLS
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

▓▓▓ Detects: division by variable, float equality, modulo hazards
Guard divisors and avoid exact float equality.

• Division by variable (possible ÷0)
⚠ Warning (999 found)
Division by variable - verify non-zero
Guard: raise if denom.zero?
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/FindEncodingProblems.rb:9 (

OpenStudio/developer/ruby/FindEncodingProblems.rb

Line 9 in c7f13ad

if /build/.match(p) || /style/.match(p)

)
if /build/.match(p) || /style/.match(p)
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/FindEncodingProblems.rb:16 (

OpenStudio/developer/ruby/FindEncodingProblems.rb

Line 16 in c7f13ad

if /BOM/.match(output)

)
if /BOM/.match(output)
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/AnalyzeDumpbin.rb:1 (

OpenStudio/developer/ruby/AnalyzeDumpbin.rb

Line 1 in c7f13ad

# to run this script, change directories to /build/src and call it

)
# to run this script, change directories to /build/src and call it
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/AnalyzeDumpbin.rb:9 (

OpenStudio/developer/ruby/AnalyzeDumpbin.rb

Line 9 in c7f13ad

    
           excludes = [/boost-log/, /expat/, /gtest/, /libssh/, /litesql/, /qwt/, /sqlite/]

)
excludes = [/boost-log/, /expat/, /gtest/, /libssh/, /litesql/, /qwt/, /sqlite/]
/Users/achapin/OpenStudio/openstudio-full/OpenStudio/developer/ruby/AnalyzeDumpbin.rb:24 (https://github.com/NREL/OpenStudio/blob/c7f13ad61579ceacf4fbe742b9a6e0c71a14cb4d/developer/r...

Fixes Bugs from Ultimate Bug Scanner #5530

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Replace rescue Exception with rescue StandardError (13 instances) - Replace bare rescue with rescue StandardError (5 instances) - Fix resource lifecycle issue in BlameFiles.rb - use File.open with block Co-authored-by: anchapin <[email protected]>

- Replace == nil and != nil with .nil? and !.nil? (30+ instances) - Fixed in SwigWrapToRDoc.rb, openstudio_cli.rb, and other files - More idiomatic Ruby and better performance Co-authored-by: anchapin <[email protected]>

Initial plan

a8958a2

Copilot AI assigned Copilot and anchapin Nov 19, 2025

Copilot started work on behalf of anchapin November 19, 2025 04:57 View session

Copilot AI and others added 2 commits November 19, 2025 05:03

Replace nil equality checks with .nil? method

17eb34a

- Replace == nil and != nil with .nil? and !.nil? (30+ instances) - Fixed in SwigWrapToRDoc.rb, openstudio_cli.rb, and other files - More idiomatic Ruby and better performance Co-authored-by: anchapin <[email protected]>

Copilot AI changed the title ~~[WIP] Fix nil equality issues detected by Ultimate Bug Scanner~~ Fix critical exception handling and Ruby idiom violations Nov 19, 2025

Copilot AI requested a review from anchapin November 19, 2025 05:11

Copilot finished work on behalf of anchapin November 19, 2025 05:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix critical exception handling and Ruby idiom violations #5531

Fix critical exception handling and Ruby idiom violations #5531

Copilot AI commented Nov 19, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix critical exception handling and Ruby idiom violations #5531

Are you sure you want to change the base?

Fix critical exception handling and Ruby idiom violations #5531

Conversation

Copilot AI commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Changes

Pull Request Author

Review Checklist

<issue_title>Bugs from Ultimate Bug Scanner</issue_title> <issue_description>Issue overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Nov 19, 2025 •

edited

Loading

<issue_title>Bugs from Ultimate Bug Scanner</issue_title>
<issue_description>Issue overview